[PATCH 5/5] winegstreamer: Use the source file size to compute duration.

[Anton Baskanov]

On пятница, 2 июля 2021 г. 04:05:12 +07 you wrote:
> On 6/21/21 9:35 AM, Anton Baskanov wrote:
> > Seems like GST_FORMAT_BYTES duration queries are forwarded all the way
> > up to my_src, which returns the source file size. On the other hand,
> > the convert query on stream->their_src expects the size of the decoded
> > raw data. This patch uses parser->their_sink to convert the source file
> > size to get a correct duration estimate.
> 
> Sorry for the complete silence. This patch set, and this patch in
> particular, has just been sitting there bothering me. I suspect that
> even if this patch set is correct it needs an audit of GStreamer
> elements to make sure it's always doing the right thing.
> 
> The basic problem is twofold: firstly, that GStreamer duration and
> convert queries are underspecified; and secondly, that the apparent
> actual behaviour is counterintuitive.
> 
> For example, I'd expect that a GST_FORMAT_TIME duration query should
> retrieve the actual running time of a stream (which might be different
> from the running time of a muxed file!), and that a GST_FORMAT_BYTES
> duration query should retrieve the total *output* size, in bytes, of a
> stream. But the default behaviour for any pad is to cheerfully ask the
> upstream filter, and GstBaseParse even explicitly does this *before*
> returning its own answers, so it'll end up returning the encoded size or
> even the whole (possibly muxed) file size. A similar problem occurs with
> GST_QUERY_CONVERT.
> 
> The ultimate effect is that the application can't predict who will
> respond to a duration or convert query, and thus they have no idea how
> to interpret the data they get back. In the case of DURATION +
> GST_FORMAT_TIME I suspect most applications get away with it, because
> usually the duration of a stream is (or is close enough to) the duration
> of a file. But GST_FORMAT_BYTES and any variation of CONVERT just seems
> completely broken...
> 
> Unfortunately, I'm not sure that downstream CONVERT queries are any
> better. The default behaviour is still to pass the query to a random
> src, and the first two demuxers I checked (avidemux and mpegpsdemux)
> both do that. I think both actually give us a correct duration in
> response to GST_QUERY_DURATION + GST_FORMAT_TIME, but it's not a good sign.
> 
> It may end up being best just to revert this path entirely, and rely on
> polling for GST_QUERY_DURATION + GST_FORMAT_TIME. The path was added by
> 613446d018. Looking back at mail history, I think I wrote it in response
> to [1], so it was actually just for dlls/quartz/tests/test.mp3.
> 
> It turns out I only got away with this because, in the case of mp3,
> there's no container, so the byte length of the encoded stream is the
> byte length of the file, mpg123 just happened to handle neither of these
> queries, and mpegaudioparse handled GST_QUERY_CONVERT in the intuitive way.
> 
> Unfortunately, unless I misread, I think there's currently no reliable
> way to get the duration of a CBR file from mpegaudioparse, except by
> relying on GST_QUERY_CONVERT.

AFAICS, mpegaudioparse should be able to get the duration of CBR files just fine 
since GstBaseParse provides a duration estimate every 50 frames until a 
subclass sets the actual duration. The only case where we have to fall back to 
GST_QUERY_CONVERT is a very short CBR file with GStreamer versions before 
1.5.90. I don't know if these old versions are still relevant, though.

> 
> So, what do we do going forward? Unfortunately I think this is the point
> where we need to ask GStreamer for help, ideally to clarify their
> documentation, and perhaps fix elements accordingly. And if we can't
> reliably use GST_QUERY_CONVERT, we may want to revert 613446d018
> entirely, and try to fix mpegaudioparse to give us a sane duration. And
> the latter seems like probably a good idea anyway.
> 
> The rest of this series looks mostly correct, but patch 2/5 is tricky,
> because it's also not clear to me from the GStreamer documentation
> whether duration-changed will always be sent if the duration isn't
> immediately available. And if we need to ask for clarification anyway,
> it would probably be good to get that clarified at the same time.
> 
> Anyway, I'll start writing an email to gstreamer-devel soon.
> 
> ἔρρωσθε,
> Zebediah
> 
> [1] https://www.winehq.org/pipermail/wine-devel/2020-June/167776.html