After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 570975 - Enable DVD Muxer
Enable DVD Muxer
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other Linux
: Normal enhancement
: 0.10.7
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-02-08 18:00 UTC by Jordi Mas
Modified: 2009-03-05 07:55 UTC
See Also:
GNOME target: ---
GNOME version: 2.23/2.24


Attachments
Proposed patch (1.28 KB, patch)
2009-02-08 18:01 UTC, Jordi Mas
none Details | Review
Reworked patch (4.00 KB, patch)
2009-02-10 20:42 UTC, Jordi Mas
needs-work Details | Review
Reworked patch (4.64 KB, patch)
2009-02-22 22:47 UTC, Jordi Mas
committed Details | Review

Description Jordi Mas 2009-02-08 18:00:36 UTC
Hello,

I working on a DVD authoring tool[1] that uses GStreamer to produce videos from sets of images. To generate DVD-Video compatible MPEG2 streams that can also be used by dvd-author is necessary to use ffmpeg dvd muxer that is not enabled in gst-ffmpeg.

Additionally, two default settings (preload & max_delay) are required by default by the DVD muxer to produce DVD compatible MPEG2. Perhaps you want to consider converting these to caps, but I do not have any use case for these values to be different.

Attached you have a patch for review that enables the dvd muxer in gst-ffmpeg. I ask you please to review it and commit to trunk.

Thanks,

Jordi,
jordimash@gmail.com

[1] http://code.google.com/p/mistelix/
Comment 1 Jordi Mas 2009-02-08 18:01:56 UTC
Created attachment 128238 [details] [review]
Proposed patch
Comment 2 Edward Hervey 2009-02-09 15:00:34 UTC
Those two settings (preload and max_delay) should be exposed as properties... once we figure out what they actually mean and what their unit is.
Comment 3 Jordi Mas 2009-02-10 20:40:47 UTC
Thanks for the feedback.

I have reworked the patch following your indications. There are two properties (preload and maxdelay) that set the correspondent fields in the underlying AVFormatContext structure[1] used by ffmpeg libraries. These are units of time  expressed in AV_TIME_BASE. 

Both properties are ints (as in the underlying structure) and I used the same description and types that they have in ffmpeg. 

Attached you have a new version of the patch.

Jordi,

[1] http://cekirdek.pardus.org.tr/~ismail/ffmpeg-docs/structAVFormatContext.html
Comment 4 Jordi Mas 2009-02-10 20:42:15 UTC
Created attachment 128412 [details] [review]
Reworked patch
Comment 5 Sebastian Dröge (slomo) 2009-02-22 18:34:35 UTC
This patch needs some more work: You need to make sure that the context is already initialized otherwise this will crash. Or better store the properties inside the instance struct and set them in the context whenever the context is created.
Comment 6 Jordi Mas 2009-02-22 22:47:34 UTC
Created attachment 129287 [details] [review]
Reworked patch

Reworked patch following Sebastian indications.

Thanks for your feedback
Comment 7 Sebastian Dröge (slomo) 2009-02-24 13:15:55 UTC
Hm, thinking about it the GObject properties should get a better time unit, like milliseconds or GstClockTime (i.e. nanoseconds) instead of AV_TIME_BASE.
Comment 8 Jordi Mas 2009-02-24 19:50:47 UTC
AV_TIME_BASE are equivalent microseconds (10 power of 6). People that has been exposed to ffmpeg is familiar with this. We can:

a) Document the unit when we setup the property, specifying clearly the unit expected for the property.

b) Convert it to milliseconds or nanoseconds or other time unit.

I actually prefer to do a). However, if you think that is more consistant with GStreamer to do b) please detail exactly which unit do you think that is better and it will made the appropriated changed to my patch.

Thanks Sebastien

Jordi,
Comment 9 Edward Hervey 2009-03-05 07:55:46 UTC
The patch committed is the same, except that I specified in the property description that the units were microseconds.

commit c7458545f301554d14cbdc04b6b1a216b0719a81
Author: Jordi Mas <jordimash@gmail.com>
Date:   Thu Mar 5 08:52:11 2009 +0100

    gstffmpegmux: Expose the 'preload' and 'maxdelay' properties, add dvd mux mapping.
    
    Fixes #570975