winegstreamer: Use provided PTS and duration in video_decoder.
This MR modifies winegstreamer to match Windows behaviour in that:
- the video decoders will output the PTS and duration of the input sample (if provided); and
- the WMV decoder will set any value not provided to zero
It also adds support for supplying a DTS value to the MFTs.
I've marked this as draft as it fixes the tests in MR !7563 (merged) (in addition to fixing some existing test_wmv_decoder
tests). Also, as demonstrated in MR !7569 (merged), our demuxers output different timestamps to Windows. This change will result in those different timestamps being forwarded from the decoder. So we may also want to address that difference prior to accepting this MR.
Merge request reports
Activity
requested review from @nsivov
added 41 commits
-
152a041b...0927c5c3 - 40 commits from branch
wine:master
- e7b93a76 - winegstreamer: Use provided PTS and duration in video_decoder.
-
152a041b...0927c5c3 - 40 commits from branch
MR !7563 (merged) has been merged, so I've marked this MR as "ready".
Edited by Brendan McGrathadded 251 commits
-
e7b93a76...099e3598 - 250 commits from branch
wine:master
- 452249a6 - winegstreamer: Use provided PTS and duration in video_decoder.
-
e7b93a76...099e3598 - 250 commits from branch
When testing these changes with FFmpeg as the demuxer, I found FFmpeg can produce negative PTS and DTS. And then on testing Windows, I discovered that Windows supports negative PTS and DTS, however, gstreamer does not.
So I've made some adjustments to this MR:
- To add tests for negative PTS and DTS; and
- To convert PTS and DTS to be zero or above on input to gstreamer and then convert back the output so that our MFTs can support negative values
This is in the MFT, which can be used by the application directly. In theory, they can assign any value they want (which is how I test the negative values on Windows).
But in terms of media data, I'll use the MPEG4 container as an example (as that's the one where I saw FFmpeg produce negative values). You get given three values/tables:
The composition offset is only used when you have a codec that needs decoding in a different order than presentation. For example, H.264 which uses B-Frames.
And it seems Windows, GStreamer and FFmpeg all use these values differently and produce different results.
As an example, say we have:
- Media Time = 0.666666s
- Time to Sample 1 = 0s
- Composition Offset for Sample 1 = 0.333333s
DTS is calculated as PTS minus composition offset.
Windows asserts, due to a Media Time of 0.666666s, then that will be the first PTS:
PTS = 0.666666s, DTS = 0.333333
GStreamer asserts that the first DTS will be 0s:
PTS = 0.333333s, DTS = 0s
FFmeg assets that the first PTS will be 0s:
PTS = 0, DTS = -0.333333s
In addition, I've seen FFmpeg produce a negative PTS on audio, but I'll have to double check the code to see how it got there. But I did note a couple of possibilities:
- The value was the negative value of Media Time;
- The value was the negative value of the duration, and the packet was flagged with
AV_PKT_FLAG_DISCARD
added 194 commits
-
8de11a12...78fe1014 - 192 commits from branch
wine:master
- dc3d601a - mf/tests: Add negative timestamp tests for h264.
- 4618f0a6 - winegstreamer: Use provided PTS and duration in video_decoder.
-
8de11a12...78fe1014 - 192 commits from branch
requested review from @rbernon
22 22 DEFINE_GUID(CLSID_decodebin_parser, 0xf9d8d64e, 0xa144, 0x47dc, 0x8e, 0xe0, 0xf5, 0x34, 0x98, 0x37, 0x2c, 0x29); 23 23 DEFINE_GUID(CLSID_mpeg_layer3_decoder, 0x38be3000, 0xdbf4, 0x11d0, 0x86, 0x0e, 0x00, 0xa0, 0x24, 0xcf, 0xef, 0x6d); 24 24 DEFINE_GUID(WINESUBTYPE_Gstreamer, 0xffffffff, 0x128f, 0x4dd1, 0xad, 0x22, 0xbe, 0xcf, 0xa6, 0x6c, 0xe7, 0xaa); 25 DEFINE_GUID(MFSampleExtension_WG_Timestamp, 0x42554ce2, 0xed8c, 0x43ee, 0x8a, 0x04, 0xd6, 0xa1, 0x68, 0xf1, 0x8f, 0xee); Oops - sorry. I got the GUID mixed up with
timestamp/x-wg-transform
.The GUID might not be necessary actually. I might be able to use the
preserve_timestamps
boolean instead. I think the only difference would be that if GStreamer lost thetimestamp/x-wg-transform
state, we would fall back to using the gstreamer values. Which is possibly better.Oops (again). I just tried using the
preserve_timestamps
boolean and it doesn't work. Thetimestamp/x-wg-transform
is a flag that letsvideo_decoder.c
know that an input timestamp has been provided (and that it should use it). It will otherwise fall back to the behavior defined byprovide_timestamps
.But if we want to get rid of the need for the GUID, I guess we could move the existing logic in
video_decoder.c
(for generating our own timestamp) in towg_transform.c
.Hmm, I'm not sure that moving timestamp generation to the unix side is better.
Can't we simply fill the sample timestamps from the input provided timestamps instead of the GStreamer-generated timestamps we don't really care about?Edited by Rémi BernonI did think of another way to do this; I could simply return another value from
wg_transform_read_mf
(in addition to the sample) to indicate that a timestamp has been provided. We could potentially even use theflags
variable that is already being passed. Let me know if you'd prefer one of those options.
179 179 WG_SAMPLE_FLAG_HAS_DURATION = 4, 180 180 WG_SAMPLE_FLAG_SYNC_POINT = 8, 181 181 WG_SAMPLE_FLAG_DISCONTINUITY = 0x10, 182 WG_SAMPLE_FLAG_HAS_DTS = 0x20, 183 WG_SAMPLE_FLAG_WG_TIMESTAMP = 0x40, 182 184 }; 183 185 184 186 struct wg_sample 185 187 { 186 188 /* timestamp and duration are in 100-nanosecond units. */ 187 UINT64 pts; 188 UINT64 duration; 189 INT64 pts; 190 INT64 duration; 191 INT64 dts; Good question. Unfortunately I must admit to not being an expert on that. Obviously it seems to decode fine without it, so it may be more useful for muxing than decoding. Although, the only codec with B-Frames that I've tested is H.264; maybe there's another codec that's more dependent on it. But I'll experiment and see if I can discern any difference between its inclusion vs. exclusion.
863 866 buffer_add_video_meta(buffer, &video_info); 864 867 } 865 868 869 /* we perform DTS first as it should never be later than PTS, so any required timestamp offset should be the maximum here */ 870 if (sample->flags & WG_SAMPLE_FLAG_HAS_DTS) 871 { 872 dts = sample->dts += transform->ts_offset; 873 if (dts < 0) 874 { 875 transform->ts_offset -= dts; changed this line in version 6 of the diff
867 GST_BUFFER_PTS(buffer) = sample->pts * 100; 881 { 882 pts = sample->pts += transform->ts_offset; 883 if (pts < 0) 884 { 885 transform->ts_offset -= pts; 886 pts = 0; 887 } 888 GST_BUFFER_PTS(buffer) = pts * 100; 889 890 if (transform->attrs.preserve_timestamps) 891 { 892 transform_timestamp = gst_caps_new_empty_simple("timestamp/x-wg-transform"); 893 gst_buffer_add_reference_timestamp_meta(buffer, transform_timestamp, pts * 100, 894 sample->flags & WG_SAMPLE_FLAG_HAS_DURATION ? sample->duration * 100 : GST_CLOCK_TIME_NONE); 895 gst_caps_unref(transform_timestamp); added 81 commits
-
4618f0a6...0e7654e5 - 79 commits from branch
wine:master
- 2cf590fd - mf/tests: Add negative timestamp tests for h264.
- ed9069fe - winegstreamer: Use provided PTS and duration in video_decoder.
-
4618f0a6...0e7654e5 - 79 commits from branch
added 1 commit
- 2be9abb9 - winegstreamer: Use provided PTS and duration in video_decoder.
added 1 commit
- b544981c - winegstreamer: Use provided PTS and duration in video_decoder.