Re: [PATCH 1/7] winegstreamer: Emit absolute timestamp in media source.

[Giovanni Mascellani]

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;