GNOME Bugzilla – Bug 526481
avidemux, qtdemux, dvdec: signal DVCPRO50 codec properly
Last modified: 2018-11-03 14:38:16 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
Created attachment 108713 [details] [review] add dv5n and dv5p to qtdemux
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.
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
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 :)
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
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
dvdec currently also does not specify any specific dvversion, so it'll be able to handle all versions. It needs a patch there too.
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
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?
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.
I wonder why we have dvdec with a higher priority than ffdec_dvvideo, which is so much faster and supports more formats...
> 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.
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?)
A second vote for decreasing the rank of dvdec.
Let's just fix this properly in 0.11, it should work acceptable now with ffdec_dvvideo having a higher rank than dvdec.
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
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
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
Ping? :)
-- 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.