GNOME Bugzilla – Bug 344937
[oggdemux] add support for queries in BYTES format
Last modified: 2018-11-03 11:12:56 UTC
oggdemux currently cannot handle queries in formats other than GST_FORMAT_TIME.
Created attachment 67373 [details] [review] Patch to add other formats than GST_FORMAT_TIME (1) This patch lets you query for other formats than GST_FORMAT_TIME. (2) The direction of the source pad is set using the property instead of directly modifying the field.
I'm not 100% sure this is correct for demuxers. If the ogg stream contains several audio/video streams, should a duration query in BYTES return the duration of the individual stream (associated to the pad on which the query was received) or the upstream duration (which in this case would make this patch correct) ?
I am not convinced this is correct either. If at all, a demuxer should IMHO return the stream length in bytes for the sub-stream corresponding to the pad queried and not the upstream length in bytes for the entire stream, just like audio decoders would return a value like number_of_samples*sample_size rather than the upstream length of the encoded stream in bytes ...
Created attachment 67398 [details] Patch to add other formats than GST_FORMAT_TIME I fully agree with you. Here's the patch.
> Created an attachment (id=67398) > > Patch to add other formats than GST_FORMAT_TIME > > I fully agree with you. Here's the patch. Do you actually get useful return values for a BYTES duration query with that patch? $ grep 'bytes =' ~/gst-plugins-base/ext/ogg/gstoggdemux.c | grep -v packet gives a hint. Basically, that kind of information (sizes of individual streams in bytes) can't be obtained without scanning the entire stream from start to end, which isn't done yet (and I can't see it being done without a good reason any time soon either).
Ok, we don't know the exact number of bytes. But we can estimate and submit a better estimation afterwards (GST_MESSAGE_DURATION). That's not yet done, but I'll work on that if you agree on the concept: A query on a pad returns the figures of that pad (and only that pad). This definition would be highly useful for the tar plugin (#303975). That's why I submitted this bug report.
> Ok, we don't know the exact number of bytes. But we can estimate and submit a > better estimation afterwards (GST_MESSAGE_DURATION). Messages are posted on the bus and are meant for the application (or to notify pipelines to invalidate any cached durations they might hold, although that is not yet implemented AFAIK), they won't be seen by downstream elements like the elements from your tar plugin. I don't think posting duration messages with BYTE durations on the bus is useful (even more so as they don't relate to the individual streams either) > That's not yet done, but I'll work on that if you agree on the concept: > A query on a pad returns the figures of that pad (and only that pad). > > This definition would be highly useful for the tar plugin (#303975). That's why > I submitted this bug report. Could you expand on how/where this would be useful for the tar plugin? And where oggdemux fits into this? Why would you want to tar up raw vorbis/theora streams? And why can't this estimating be done in the tar elements (or why would it be better not done there but in oggdemux?)
Both oggdemux and tardemux create one pad per stream. When playing one file in a TAR-file, I'd like to know the duration of this file (not the total duration of all files in the TAR-file). That is, the duration of the pad currently playing. The same applies for querying the position. I am just pointing out the similarities between the two elements. I am not talking about linking them.
> I am just pointing out the similarities between the two elements. I am not > talking about linking them. I still don't understand why oggdemux should be answering BYTE format queries on its source pads, even more so with estimated values.
If you don't support the query in BYTE format, frontents will never be able to display an error message like "You don't have enough disk space to perform this operation." That's the idea.
bytes query should indeed be supported. unfortunatly we don't yet return any indication about the accuracy of the result so that it is of.rather limited use. sending a duration message invalidates any cache and can be used to notify an update.
the patch should return the number of bytes of that stream, not the total lenght.seems like it would be rather hard to do this correctly unless you can get or guess the bps of the ogg streams somehow. care to prepare another patch?
Created attachment 76180 [details] [review] Some ideas for cleaning up Should I put the estimation logic into GstPad? I.e. keep track of the average byte output per second if the only format supported is GST_FORMAT_TIME and use that average to provide an estimation if the pad gets queried for GST_FORMAT_BYTES?
Created attachment 76355 [details] [review] Try to estimate query results if query failed This patch tries to estimate the query result (GST_QUERY_[DURATION,POSITION] / GST_FORMAT_PERCENT) if querying the pad failed. Like that, we could implement the query in BYTES format as an estimation. Should I?
As there is another patch now, I am reopening.
What's the state of this bug? Wim, the second patch (for gstpad.c) looks very useful to me and should probably committed. It would finally allow to use queries in PERCENT format. For the BYTES queries I don't think it's a good idea to a) forward them upstream in oggdemux (upstream doesn't know the bytes of a single stream inside the ogg container) and b) we can't/shouldn't do estimations inside gstpad.c because everything currently assumes that BYTES queries return accurate results. It might make sense to add a "accuracy" field to the position/duration queries though to get around this.
Ping?
Not very keen on this, also not the gstpad.c patch. Would just WONTFIX really.. An accuracy indication for position and especially duration queries would be useful in general though. It's not clear to me that a length in BYTES should ever be estimated. It rarely makes sense. If you want PERCENT support, implement it properly in the relevant places (basesrc, demuxers, baseparse), instead of doing fallback magic in gstpad.c.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/5.