After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769392 - flacparse: add maximum bitrate tag
flacparse: add maximum bitrate tag
Status: RESOLVED WONTFIX
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal enhancement
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-08-01 16:52 UTC by Carlos Rafael Giani
Modified: 2016-08-18 11:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Additional PCM-like maximum bitrate tag (1.15 KB, patch)
2016-08-01 16:53 UTC, Carlos Rafael Giani
committed Details | Review

Description Carlos Rafael Giani 2016-08-01 16:52:12 UTC
The maximum possible bitrate with a FLAC file is the bitrate of the uncompressed audio stream. Add this as the maximum bitrate tag to inform downstream what the worst case is. This is useful for network buffer size computations.
Comment 1 Carlos Rafael Giani 2016-08-01 16:53:18 UTC
Created attachment 332489 [details] [review]
Additional PCM-like maximum bitrate tag
Comment 2 Sebastian Dröge (slomo) 2016-08-02 06:27:59 UTC
Review of attachment 332489 [details] [review]:

::: gst/audioparsers/gstflacparse.c
@@ +1711,3 @@
+    gst_tag_list_add (flacparse->tags, GST_TAG_MERGE_KEEP,
+        GST_TAG_MAXIMUM_BITRATE, flacparse->samplerate * flacparse->bps *
+        flacparse->channels, NULL);

Is that so? The worst case should be even higher than that. But I guess this is a valid guesstimate
Comment 3 Sebastian Dröge (slomo) 2016-08-02 11:37:03 UTC
commit c703ab69f526092bb26cce41ca691a896c8383d8
Author: Carlos Rafael Giani <dv@pseudoterminal.org>
Date:   Mon Aug 1 18:52:26 2016 +0200

    flacparse: Add maximum bitrate tag
    
    https://bugzilla.gnome.org/show_bug.cgi?id=769392
Comment 4 Tim-Philipp Müller 2016-08-02 13:30:16 UTC
I think this is a bit bogus tbh, are we going to add max bitrate tags for everything like that? video streams then width*height*4*framerate etc?
Comment 5 Carlos Rafael Giani 2016-08-02 13:36:56 UTC
In our software here we actually had a special case for lossless audio like FLAC that uses a PCM-like bitrate for buffer size estimations. It is the safest bet with lossless audio, since average bitrates have a variance that is far too large. Extreme cases can happen, like a FLAC that has silence in the beginning, leading to a network buffer that is way too small.
Comment 6 Tim-Philipp Müller 2016-08-05 00:34:39 UTC
I'm sure it's useful to you, but I am not sure it's really in line with the semantics of the bitrate tags to make up things like this, even just limited to lossless audio.
Comment 7 Carlos Rafael Giani 2016-08-05 11:37:08 UTC
It is useful for proper buffering without interruptions during playback. But, I am inclined to agree that perhaps we should revert this, for one simple reason:

If we apply this rule (= add PCM-like maximum bitrate tag to any lossless audio format because of the high avg bitrate variance over time), then we'd have to do the same for any lossy format that can operate in a VBR manner. Which are most popular formats (MP3, AAC, Vorbis ..). And there's the equivalent in video.

So, as an alternative, it would make more sense to make the bitrate-based size estimation in uridecodebin (and urisourcebin) more robust I guess.
Comment 8 Sebastian Dröge (slomo) 2016-08-09 06:32:23 UTC
So let's revert this one?
Comment 9 Sebastian Dröge (slomo) 2016-08-18 09:54:40 UTC
Ping? :)
Comment 10 Tim-Philipp Müller 2016-08-18 11:04:06 UTC
Done:

commit 0f41d0e75dab024d6d79864361e37b5d9199c4c0
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Thu Aug 18 12:02:01 2016 +0100

    Revert "flacparse: Add maximum bitrate tag"
    
    This reverts commit c703ab69f526092bb26cce41ca691a896c8383d8.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=769392


Let's try to fix whatever the problem is here in some other way then :)