From: Giovanni Mascellani Subject: Re: [PATCH 1/7] winegstreamer: Emit absolute timestamp in media source. Message-Id: <479e88eb-fb0e-0342-b9e2-321905664f95@codeweavers.com> Date: Fri, 17 Sep 2021 16:35:45 +0200 In-Reply-To: <20210906151109.225515-1-gmascellani@codeweavers.com> References: <20210906151109.225515-1-gmascellani@codeweavers.com> Hi, this patch is unfortunately getting little attention, so I'll try to add a little bit of context in the hope to convince somebody to have a look at it. Each sample emitted by a media source has attached a timestamp (often called "presentation timestamp", or PTS), which is used by the sink that will receive the sample to ensure that it is rendered at the right time. The current Wine behavior is to emit a presentation timestamp relative to the moment where playback was last started or restarted. I.e., the frame that sits two seconds in the media file would get PTS = 2 when playback started from beginning, but PTS = 1 if, for instance, playback was stopped, seeked to 1 second in the file and then restarted. This behavior is, according to my tests, wrong. Windows always emits PTS = 2 for the frame two seconds in the file, independently of the point at which playback started. I have some tests that show this in the later patches of this set. As per the subthread with Nikolay, they are not submitted for inclusion in Wine, but to ease review of the first three patches (1/7 through 3/7). You can see at https://testbot.winehq.org/JobDetails.pl?Key=97478 that the tests are correct on Windows (except for Windows 7, where the test program crashed to to a bug in Windows 7 that was later solved). You can also check that some failures appear if you run the tests in 4/7 through 7/7 and revert this patch. The actual content of this patch is pretty simple: when GStreamer returns a buffer, the PTS of that buffer is recovered in wg_parser.c (using the macro GST_BUFFER_PTS) and stored in the field event->u.buffer.pts. The previous implementation used to store also the timestamp at which the file was last restarted (stored in the start_time field of struct media_source), and subtracted that value from the PTS returned by GStreamer before passing it to SetSampleTime. With this change the subtraction is removed, as is the start_time field which is now useless. Giovanni. Il 06/09/21 17:11, Giovanni Mascellani ha scritto: > Signed-off-by: Giovanni Mascellani > --- > dlls/winegstreamer/media_source.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c > index 01ab626254a..cd8957db7b1 100644 > --- a/dlls/winegstreamer/media_source.c > +++ b/dlls/winegstreamer/media_source.c > @@ -100,8 +100,6 @@ struct media_source > SOURCE_SHUTDOWN, > } state; > > - LONGLONG start_time; > - > HANDLE read_thread; > bool read_thread_shutdown; > }; > @@ -274,7 +272,6 @@ static void start_pipeline(struct media_source *source, struct source_async_comm > position->vt = VT_I8; > position->hVal.QuadPart = 0; > } > - source->start_time = position->hVal.QuadPart; > > for (i = 0; i < source->stream_count; i++) > { > @@ -427,7 +424,7 @@ static void send_buffer(struct media_stream *stream, const struct wg_parser_even > goto out; > } > > - if (FAILED(hr = IMFSample_SetSampleTime(sample, event->u.buffer.pts - stream->parent_source->start_time))) > + if (FAILED(hr = IMFSample_SetSampleTime(sample, event->u.buffer.pts))) > { > ERR("Failed to set sample time, hr %#x.\n", hr); > goto out; >