GNOME Bugzilla – Bug 743657
dvdemux: no tags for .dv files
Last modified: 2017-05-20 15:03:29 UTC
Created attachment 295677 [details] demos bug Trying to determine start time of .dv files, d = discoverer.discover_uri('file:///home/carl/temp/sample2.dv') d.get_tags() returns None. bilboed: (it's the dvdemux element that needs to extract the info)
Created attachment 295678 [details] 1 frame dv file used by demo program
carl@twist:~/temp$ mediainfo sample2.dv General Complete name : sample2.dv Format : DV Commercial name : DVCPRO File size : 117 KiB Duration : 33ms Overall bit rate mode : Constant Overall bit rate : 28.8 Mbps Recorded date : 2014-11-15 12:07:07.000
it should be as trivial as using the dv_get_recording_datetime() function from libdv.
Created attachment 296592 [details] [review] get recorded time from libdv I made patch for this. This showed tag recorded datetime with gst-launch '-t' option. But, discoverer didn't fetch this tag. I'll try to find out.
Any updates?
Created attachment 302512 [details] with uridecodebin dvdemux seems fine. When I tried with pipeline gst-launch filesrc location=sample.dv ! dvdemux ! fakesink, tags were received properly. However, with uridecodebin tags weren't received. Since discoverer uses uridecodebin, it didn't get the tags. Attached pic was generated by dot tool for uridecodebin use case. Also, Carl, would it be possible for you to upload file with more samples (current one has only 1 sample).
decodebin seems to be using dvdemux, any reason why it shouldn't pick up the tags when inside decodebin? Did you try reading the logs?
> > Also, Carl, would it be possible for you to upload file with more samples > (current one has only 1 sample). http://5cda49ca88af98bf1f1e-b4c3b47b38bb1b572e0805ecabeeb59c.r76.cf2.rackcdn.com/10_04_51.dv 12 meg - I hear that is too big for an attachment. $ mediainfo 10_04_51.dv General Complete name : 10_04_51.dv Format : DV Commercial name : DVCPRO File size : 11.1 MiB Duration : 3s 237ms Overall bit rate mode : Constant Overall bit rate : 28.8 Mbps Recorded date : 2015-04-13 10:04:51.000 Video Format : DV Commercial name : DVCPRO Duration : 3s 237ms Bit rate mode : Constant Bit rate : 24.4 Mbps Width : 720 pixels Height : 480 pixels Display aspect ratio : 16:9 Frame rate mode : Constant Frame rate : 29.970 fps Standard : NTSC Chroma subsampling : 4:1:1 Bit depth : 8 bits Scan type : Interlaced Scan order : Bottom Field First Compression mode : Lossy Bits/(Pixel*Frame) : 2.357 Stream size : 9.42 MiB (85%) Audio ID : 0 Format : PCM Duration : 3s 237ms Bit rate mode : Constant Bit rate : 1 536 Kbps Encoded bit rate : 0 bps Channel(s) : 2 channels Sampling rate : 48.0 KHz Bit depth : 16 bits Stream size : 607 KiB (5%)
Comment on attachment 296592 [details] [review] get recorded time from libdv ommit ebbefbb792737f5ccad94d3067c8289faedd534d Author: Ravi Kiran K N <ravi.kiran@samsung.com> Date: Wed Feb 11 18:09:24 2015 +0530 dvdemux: extract recording time Extracts the recorded time of the dv file from the metadata and puts it into the global tags. https://bugzilla.gnome.org/show_bug.cgi?id=743657
Created attachment 303403 [details] [review] push tag through video srcpad dvdemux tries to send tag to dvdemux->videosrcpad before newly created pad is assinged to it. With attached patch, tags were received by sink properly. And, signalled no-more-pads after creating both pads. Please review.
Comment on attachment 303403 [details] [review] push tag through video srcpad Doesn't this send a tag event now for every audio/video packet being demuxed?
Tag event is sent in gst_dvdemux_add_pad(). But, dvdemux->videosrcpad, dvdemux->audiosrcpad are assigned with return value of gst_dvdemux_add_pad(). So, when gst_dvdemux_demux_video() calls gst_dvdemux_add_pad() for adding videosrcpad, dvdemux->videosrcpad will still be null. So, it doesn't send tag to videosrcpad. But, it sends it to audiosrc pad, because by the time gst_dvdemux_add_pad() is called to create videosrcpad, dvdemux->audiosrcpad is already populated.
ping
push_tags() is now called from the demux_video() and demux_audio() functions so it indeed seems like it will push a tag event for every frame demuxed. Could you address this issue, please?
Created attachment 308366 [details] [review] push tag through video srcpad Thanks. I made a new patch. Pls review.
Comment on attachment 308366 [details] [review] push tag through video srcpad - it looks like "no-more-pads" will now be emitted for every dv frame - you can do this simpler: instead of adding gboolean pending_tags_[av], add a GstEvent *pending_tag_event_[av], and just set those in gst_dvdemux_add_pad(). Then in _demux_[audio|video]() you can do if (demux->pending_tag_event_[av]) { gst_pad_push_event ... ; demux->pending_tag_event_[av] = NULL; }
Created attachment 308884 [details] [review] push tag through video srcpad Thanks for the inputs. no-more-pads is now sent after creating video src pad (if both pads are created). Let me know if that should be added after creating audio src pad too.
I think it would be best not to make assumption about which pad is created first and last.
Created attachment 308993 [details] [review] push tag through both pads yes. Here is updated patch.
Review of attachment 308993 [details] [review]: ::: ext/dv/gstdvdemux.c @@ +306,3 @@ + gst_tag_list_set_scope (tags, GST_TAG_SCOPE_GLOBAL); + + if (dv_get_recording_datetime (dvdemux->decoder, rec_datetime)) { You can create the taglist only if needed here, otherwise you end with an empty one. @@ +1222,3 @@ + gst_pad_push_event (dvdemux->audiosrcpad, dvdemux->pending_tag_event); + dvdemux->pending_tag_event = NULL; + } You can push the event directly on the add_pad function as the tag event is sticky and will be forwarded once the pad is linked and data is pushed. Also you can store the tag event in the demuxer and just push it again when a new pad is needed, otherwise you will end up creating the same object twice from scratch. Just remember to clean it up when doing from PAUSED to READY.
Thanks, I'll make those changes. A small doubt - taglist is created with GST_TAG_CONTAINER_FORMAT ("DV") tag added. Do you suggest adding it only when DATE_TIME is avaialble?
(In reply to RaviKiran from comment #21) > Thanks, I'll make those changes. A small doubt - taglist is created with > GST_TAG_CONTAINER_FORMAT ("DV") tag added. Do you suggest adding it only > when DATE_TIME is avaialble? Good point. Just leave it outside the if, but please only create that once and re-send when a new pad is added. I'm guessing this datetime doesn't change for streams or during the stream.
Created attachment 310776 [details] [review] push tag through both pads Thanks. Pls review the new patch.
Created attachment 310777 [details] [review] push tag through both pads Changed commit msg a little.
commit de3e54690bc5b37961c166352ad10d77d06968aa Author: Ravi Kiran K N <ravi.kiran@samsung.com> Date: Sun Sep 6 20:49:59 2015 +0530 dvdemux: Push tag event to both pads Tags are pushed to "videosrcpad"/"audiosrcpad" in gst_dvdemux_add_pad() method, however they will be NULL in this method, hence tags are not pushed. Instead, send tag event to "pad" created gst_dvdemux_add_pad(). Signal no-more-pads when both pads are created https://bugzilla.gnome.org/show_bug.cgi?id=743657 tpm@xps:~/gst/master/gst-plugins-good/ext/dv$ git push