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 756967 - matroskamux: drops JPEG input buffers with just PTS and no DTS set on them
matroskamux: drops JPEG input buffers with just PTS and no DTS set on them
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.6.0
Other Linux
: Normal major
: 1.6.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-10-22 13:23 UTC by Srimanta Panda (trollkarlen)
Modified: 2015-10-28 19:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed fix (802 bytes, patch)
2015-10-27 09:50 UTC, Nicola
none Details | Review

Description Srimanta Panda (trollkarlen) 2015-10-22 13:23:36 UTC
From video sources producing JPEG format and only setting the PTS for the outgoing buffer (not setting the DTS), matroska muxer drops all the buffer assuming invalid time time (during gst_matroska_mux_write_data()).

The root cause of the issue is for JPEG, a dts_only flag is set to TRUE. And based on this flag, matroska tries to get the DTS as the time stamp of the buffer (matroska-ids.c:gst_matroska_track_get_buffer_timestamp()).
Comment 1 Srimanta Panda (trollkarlen) 2015-10-22 13:28:27 UTC
I am working on it.
Comment 2 Nicola 2015-10-26 17:50:32 UTC
the problem is here:

http://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/gst/matroska/matroska-ids.c#n307

jpeg is muxed as V_MS/VFW/FOURCC so dts is expected, this behavioiur was introduced by this patch:

https://bugzilla.gnome.org/show_bug.cgi?id=745192#c9

maybe a fallback to pts could be used if dts is not valid
Comment 3 Nicola 2015-10-26 17:52:21 UTC
additionally for jpeg and raw streams muxed as V_MS/VFW/FOURCC dts and pts are the same since these are all keyframes streams
Comment 5 Edward Hervey 2015-10-27 09:31:07 UTC
Yes, that new macro is indeed meant for such use-cases
Comment 6 Nicola 2015-10-27 09:37:14 UTC
(In reply to Edward Hervey from comment #5)
> Yes, that new macro is indeed meant for such use-cases

Ok, I'll send a patch later today then
Comment 7 Nicola 2015-10-27 09:50:14 UTC
Created attachment 314201 [details] [review]
proposed fix

Srimanta, can you please confirm that this patch fix your issue?

thanks
Nicola
Comment 8 Srimanta Panda (trollkarlen) 2015-10-27 11:07:04 UTC
ok..I will check it as soon as possible and give you the feedback :)
Comment 9 Srimanta Panda (trollkarlen) 2015-10-27 13:35:49 UTC
Hi Nicola, it works now. 
That is exactly the same fix I was trying before but with out the MACRO :).
Comment 10 Nicola 2015-10-27 15:18:00 UTC
(In reply to Srimanta Panda from comment #9)
> Hi Nicola, it works now. 
> That is exactly the same fix I was trying before but with out the MACRO :).

ok thanks for confirming, so now we have to wait for developers code reviews and suggestions
Comment 11 Tim-Philipp Müller 2015-10-28 19:12:05 UTC
Should be fine, thanks for the patch.


commit 65d08e2154eba75323fa7badfb78c0c9327d0a43
Author: Nicola Murino <nicola.murino@gmail.com>
Date:   Tue Oct 27 10:48:00 2015 +0100

    matroskamux: don't drop JPEG frames that only have PTS but no DTS set
    
    For the MS/VfW codec ids, we want to write DTS timestamps instead
    of PTS because that's what everyone else seems to do (and it's also
    how it is in AVI). So for those input formats we use the buffer DTS
    instead of the PTS. However, if there's no DTS set but only the PTS
    then just take the PTS instead of dropping the input buffer. This
    is useful especially for I-frame only codecs like JPEG and huffyuv,
    but should also be fine as fallback in general.
    
    Fixes regression with input JPEG frames that only have PTS set on them.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=756967