[PATCH 2/2] winegstreamer: Continuously query duration for mp3 audio streams.

[Zebediah Figura (she/her)]

On 5/3/21 7:53 AM, Connor McAdams wrote:
> For mp3 audio streams, continuously call gst_pad_query_duration until
> the duration arrives. Fixes issue of incorrect song duration in Fallout:
> New Vegas.
> 
> Signed-off-by: Connor McAdams <cmcadams at codeweavers.com>
> ---
>   dlls/winegstreamer/wg_parser.c | 27 +++++++++++++++++++++++++--
>   1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c
> index 1653701506c..1dae3bfb922 100644
> --- a/dlls/winegstreamer/wg_parser.c
> +++ b/dlls/winegstreamer/wg_parser.c
> @@ -1504,13 +1504,22 @@ static gboolean src_event_cb(GstPad *pad, GstObject *parent, GstEvent *event)
>       return ret;
>   }
>   
> -static LONGLONG query_duration(GstPad *pad)
> +static LONGLONG query_duration(GstPad *pad, gboolean query_duration)
>   {
>       gint64 duration, byte_length;
>   
>       if (gst_pad_query_duration(pad, GST_FORMAT_TIME, &duration))
>           return duration / 100;
>   
> +    if (query_duration)
> +    {
> +        for (;;)
> +        {
> +            if (gst_pad_query_duration(pad, GST_FORMAT_TIME, &duration))
> +                return duration / 100;
> +        }
> +    }
> +

We don't want to risk an infinite loop here, or busy-wait. We should 
wait for duration-changed with a time-out.

We also should be calling query_duration when polling, rather than 
gst_pad_query_duration(). That is, this loop should be in 
wg_parser_connect().

>       WARN("Failed to query time duration; trying to convert from byte length.\n");
>   
>       /* To accurately get a duration for the stream, we want to only consider the
> @@ -1569,6 +1578,7 @@ static HRESULT CDECL wg_parser_connect(struct wg_parser *parser, uint64_t file_s
>       for (i = 0; i < parser->stream_count; ++i)
>       {
>           struct wg_parser_stream *stream = parser->streams[i];
> +        gboolean duration_wait = FALSE;
>   
>           while (!stream->has_caps && !parser->error)
>               pthread_cond_wait(&parser->init_cond, &parser->mutex);
> @@ -1577,12 +1587,25 @@ static HRESULT CDECL wg_parser_connect(struct wg_parser *parser, uint64_t file_s
>               pthread_mutex_unlock(&parser->mutex);
>               return E_FAIL;
>           }
> +
> +        /*
> +         * On MP3 audio streams, sometimes the duration value takes time to be
> +         * set. In this case, we need to continuously call query_duration
> +         * until we get it.
> +         */
> +        if (input_format.major_type == WG_MAJOR_TYPE_AUDIO &&
> +            input_format.u.audio.format >= WG_AUDIO_FORMAT_MPEG1_LAYER1 &&
> +            input_format.u.audio.format <= WG_AUDIO_FORMAT_MPEG1_LAYER3)
> +        {
> +            duration_wait = TRUE;
> +        }
> +

I don't think there's really a good reason to limit this to MPEG audio. 
If there are other demuxers (now or in the future, or even in the past) 
that force us to wait for duration, we want to do so.

>           /* GStreamer doesn't actually provide any guarantees about when duration
>            * is available, even for seekable streams. However, many elements (e.g.
>            * avidemux, wavparse, qtdemux) in practice record duration before
>            * fixing caps, so as a heuristic, wait until we get caps before trying
>            * to query for duration. */
> -        stream->duration = query_duration(stream->their_src);
> +        stream->duration = query_duration(stream->their_src, duration_wait);
>       }
>   
>       pthread_mutex_unlock(&parser->mutex);
>