GNOME Bugzilla – Bug 746017
audiodecoder: Take into account subclass PTS if provided
Last modified: 2018-11-03 11:35:43 UTC
Some decoder implementations operate with a different thread and know best what output data correspond to which input data. for those implementation (such as the androidmedia audio decoder), allow subclasses to provide a timestamp for the decoder buffer. The base class will stil do the various tolerance/drift/.. checks, but will use the provided timestamp instead of the (potentially bogus) head frame value.
Created attachment 299078 [details] [review] audiodecoder: Take into account subclass PTS if provided Some implementations of audio decoders might be operating with a separate thread and known themselves the best correlation between input data and output data. In those cases, allows subclasses to provide timestamps on the decoded buffers. The base class will still do the various tolerance/drift checks, but on that value instead of one the (potentially bogus) frame head timestamp
Created attachment 299079 [details] [review] amcaudiodec: Use implementation output PTS Some implementations don't provide 1-to-1 output (i.e. one output buffer for each input buffer). Instead of having the parent class trying to (wrongly) figure out the timestamp of the decoded buffer, provide the timestamp we got from the implementation. This fixes broken playback on several android devices. The baseclass will take care of jitter/drift/duration/... corrections.
Review of attachment 299078 [details] [review]: Why would the timestamp ever be different than the timestamp of the oldest pending frame? I think the problem here is more that the subclass does not use the base class properly... erm... I mean that the base class does too many assumptions and requires the subclass to do things in a very specific way (as described in the other bug). I think instead of this patch it would make more sense to unbreak the timestamp/frame tracking code in the base class.
A problem I see with this patch is that we might accumulate more and more frame data in the base class if the subclass does not properly say how many frames are finished, and if it does the timestamps should already be ok. Also if the subclass sometimes sets timestamps and sometimes not, things will probably break.
tbh, I haven't been able to figure out wth is going on with those frames. The case I have is that it's inputting frames of 21ms, but the implementation returns frames of 42ms (at half the number obviously). Is that meant to work with the current implementation ?
Yes, it's supposed to work if you call finish() with 2. amcaudiodec has some magic code (for MP3 only currently) to guess the number of samples per frame and then to guess the number of frames per output buffers.
I came across an android decoder that always returns less data for the first output buffer which makes it impossible to calculate the number of frames being finished.
(In reply to Mohammed Sameer from comment #7) > I came across an android decoder that always returns less data for the first > output buffer which makes it impossible to calculate the number of frames > being finished. See http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=8bacecfb08bf040424431feae1e6a4e64ce32f2d , which works around this problem (assuming you properly set spf for the codecs you use... which currently is only done for MP3).
(In reply to Sebastian Dröge (slomo) from comment #8) > (In reply to Mohammed Sameer from comment #7) > > I came across an android decoder that always returns less data for the first > > output buffer which makes it impossible to calculate the number of frames > > being finished. > > See > http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/ > ?id=8bacecfb08bf040424431feae1e6a4e64ce32f2d , which works around this > problem (assuming you properly set spf for the codecs you use... which > currently is only done for MP3). And that is an issue. How can one know the spf correctly? If we however take into account subclass PTS then I assume it should fix the issue.
Because you know that e.g. every MP3 frame has 1152 samples, every AAC frame (in some profile) has 1024 samples, etc. You need additional knowledge about the codec for that, which is not too bad... just suboptimal. Relying on the timestamps for that is going to never work reliable, hardware codecs often give you the most interesting values there... Of course this is all only a workaround, ideally the audio decoder base class would not care at all about how many frames an output buffer contains and just do the right thing.
-- 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/171.