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 526481 - avidemux, qtdemux, dvdec: signal DVCPRO50 codec properly
avidemux, qtdemux, dvdec: signal DVCPRO50 codec properly
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-04-06 11:18 UTC by j^
Modified: 2018-11-03 14:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add dv5n and dv5p to qtdemux (741 bytes, patch)
2008-04-06 11:18 UTC, j^
needs-work Details | Review
dvcpro50 with dvversion=25 and dvversion=50 (1.00 KB, patch)
2008-05-01 13:33 UTC, j^
none Details | Review
no dvversion=25 but dvversion=50 and 100 (1.17 KB, patch)
2008-05-01 19:23 UTC, j^
none Details | Review
and the gst-ffmpeg part (684 bytes, patch)
2008-05-01 19:24 UTC, j^
needs-work Details | Review
gst-plugins-good patch (1.97 KB, patch)
2008-05-02 14:29 UTC, j^
reviewed Details | Review

Description j^ 2008-04-06 11:18:03 UTC
add fourcc for DVCPRO50 to qtdemux
dv5n is DVCPRO50 NTSC and
dv5c is DVCPRO50 PAL

right now only ffdec_dvvideo is able to decode those files properly
but totem chooses dvdec, not sure how to fix that.
playing them via gst-launch works:
gst-launch filesrc location=test.mov ! qtdemux ! ffdec_dvvideo ! ffmpegcolorspace ! xvimagesink
Comment 1 j^ 2008-04-06 11:18:40 UTC
Created attachment 108713 [details] [review]
add dv5n and dv5p to qtdemux
Comment 2 Edward Hervey 2008-04-07 07:46:32 UTC
It would actually be good to have additional properties to the caps for DVCPro50 (and the other DV formats which aren't consumer 25Mb/s).

This would allow us to properly add those caps to the ffdec_dvvideo/ffenc_dvvideo element, but not dvdec, and therefore have proper autoplug.
Comment 3 j^ 2008-05-01 13:33:34 UTC
Created attachment 110218 [details] [review]
dvcpro50 with dvversion=25 and dvversion=50

would something like this work? other codes have codecversion.
for dv it could be something like  dvversion=25 and  dvversion=50, 
with  dvversion=100 coming soon for DVCPROHD
Comment 4 Edward Hervey 2008-05-01 18:50:30 UTC
except that the proposed patch would break current behaviour (since it's not backwards compatible with the existing consumer DV25).
We should keep the old caps for consumer DV25 (without any specified version) and use dvversion=50 for DVCPRO50.

And... it requires the symmetric patch to gst-ffmpeg so we can plug it into the appropriate decoder :)
Comment 5 j^ 2008-05-01 19:23:48 UTC
Created attachment 110239 [details] [review]
no dvversion=25 but dvversion=50 and 100

this add dvversion=50 and dvvserion=100 for DVCPro HD material, 
DVCPro HD decoder patch exists but is not in ffmpeg svn yet
Comment 6 j^ 2008-05-01 19:24:55 UTC
Created attachment 110240 [details] [review]
and the gst-ffmpeg part

make gst-ffmpeg advertise its ability to play dvversion 25 and 50
100 should be added as soon as its in ffmpeg and gst-ffmpeg uses a new version of ffmpeg
Comment 7 Wim Taymans 2008-05-02 09:17:18 UTC
dvdec currently also does not specify any specific dvversion, so it'll be able to handle all versions. It needs a patch there too.
Comment 8 j^ 2008-05-02 14:29:15 UTC
Created attachment 110273 [details] [review]
gst-plugins-good patch

this patches qtdemux, adding dvversion 25, 50 and 100
and dvdec exposing support for dvversion 25 in the caps
Comment 9 Daniel Golle 2008-05-09 13:27:48 UTC
i'm thinking about creating a wrapper around MainConcept's DVCProHD codecs (the company I'm currently working got the SDK license). just starting to investigate both, GStreamer and MainConcept's SDK. i'll work on that during the weekend.
any hints?
Comment 10 Wim Taymans 2008-05-26 16:25:45 UTC
Starrting with the qt patch should not cause problems we did not already have before.

        Patch by: j^ <j at oil21 dot org>

        * gst/qtdemux/qtdemux.c: (qtdemux_video_caps):
        Add caps for DVCPRO50 and DVCPRO HD PAL/NTSC. See #526481.
Comment 11 Wim Taymans 2008-05-26 17:55:20 UTC
I wonder why we have dvdec with a higher priority than ffdec_dvvideo, which is
so much faster and supports more formats...
Comment 12 Tim-Philipp Müller 2008-05-27 10:56:44 UTC
> I wonder why we have dvdec with a higher priority than ffdec_dvvideo, which is
> so much faster and supports more formats...

I think we did that because there were a few cases where it just crashed. 

Comment 13 Tim-Philipp Müller 2009-08-01 19:52:45 UTC
This hasn't moved in ages - anything else we can do?

Or should it be marked as [0.11] since we effectively messed up the caps/api now? (Which is less likely to get noticed because ffdec_dvvideo has a higher rank than dvdec these days, right? Does dvdec at least reject the 50 caps properly if it can't handle them?)
Comment 14 David Schleef 2009-08-01 21:28:13 UTC
A second vote for decreasing the rank of dvdec.
Comment 15 Sebastian Dröge (slomo) 2011-05-26 09:12:02 UTC
Let's just fix this properly in 0.11, it should work acceptable now with ffdec_dvvideo having a higher rank than dvdec.
Comment 16 Sebastian Dröge (slomo) 2011-05-26 09:13:30 UTC
Review of attachment 110240 [details] [review]:

::: ext/ffmpeg/gstffmpegcodecmap.c
@@ +472,3 @@
       } else {
         caps = gst_ff_vid_caps_new (context, codec_id, "video/x-dv",
+            "dvversion", GST_TYPE_INT_RANGE, 25, 50, NULL,

Shouldn't this get into two structures or a GST_TYPE_LIST instead? ffmpeg only supports 25 or 50, right?

In that case just make it a GST_TYPE_LIST, see CODEC_ID_AAC in git for an example how to do it
Comment 17 Tim-Philipp Müller 2013-03-16 15:20:12 UTC
So what's the "proper" fix then?

If we need to add a field to signal the newer variant, so be it IMHO, if that's what we need to avoid brokeness.

dvdec accepts the data and plays it in a garbled way (outputs frames with corrupted blocks).

Tested with http://samples.mplayerhq.hu/V-codecs/DV50/dvcpro50.avi
Comment 18 Sebastian Dröge (slomo) 2013-08-21 18:01:57 UTC
I'd say that would be the correct fix, yes. That's what we do with other codecs too.

Any reason not to do that here? Someone needs to update the gst-plugins-good patch though. Also everything probably doesn't apply against latest git master anymore
Comment 19 Sebastian Dröge (slomo) 2014-01-25 14:00:29 UTC
Ping? :)
Comment 20 GStreamer system administrator 2018-11-03 14:38:16 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/3.