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 599298 - [tsdemux] Emit codec tags
[tsdemux] Emit codec tags
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.1.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 563297
 
 
Reported: 2009-10-22 13:17 UTC by Bastien Nocera
Modified: 2013-08-13 14:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to post bitrate from the MPEG sequence header (1.98 KB, patch)
2010-02-02 10:34 UTC, Arun Raghavan
none Details | Review
Patch to post bitrate from the MPEG sequence header as a tag (2.40 KB, patch)
2010-02-02 13:17 UTC, Arun Raghavan
accepted-commit_now Details | Review
Patch to post bitrate from the MPEG sequence header as a tag (2.61 KB, patch)
2010-02-23 13:06 UTC, Arun Raghavan
committed Details | Review
mpegdemux: send codec tag for each stream (2.78 KB, patch)
2013-08-02 18:59 UTC, Arnaud Vrac
committed Details | Review

Description Bastien Nocera 2009-10-22 13:17:11 UTC
The nominal video bitrate and codec fields of the stream should be populated so that front-ends
can show it.
Comment 1 Zaheer Abbas Merali 2010-01-07 21:07:58 UTC
the demuxer doesn't know the bitrate of the video. the codec it does however but it makes more sense for the decoders or parsers to fill this info.
Comment 2 Arun Raghavan 2010-02-02 10:34:19 UTC
Created attachment 152822 [details] [review]
Patch to post bitrate from the MPEG sequence header
Comment 3 Sebastian Dröge (slomo) 2010-02-02 11:23:28 UTC
(In reply to comment #2)
> Created an attachment (id=152822) [details] [review]
> Patch to post bitrate from the MPEG sequence header

You should post the bitrate as tag and don't put it into the caps.

Also, this patch is for mepgvideoparse while this bug is about mpegtsdemux/mpegpsdemux :)
Comment 4 Zaheer Abbas Merali 2010-02-02 11:36:20 UTC
Maybe because I suggested it is done in the parser, not in the demuxer.
Comment 5 Arun Raghavan 2010-02-02 13:14:41 UTC
Yep, what Zaheer said. :)
Comment 6 Arun Raghavan 2010-02-02 13:17:19 UTC
Created attachment 152833 [details] [review]
Patch to post bitrate from the MPEG sequence header as a tag

Thanks for the review, Sebastian. Updating the patch to post bitrate as a tag instead of stuffing it in caps.
Comment 7 Sebastian Dröge (slomo) 2010-02-02 15:21:13 UTC
Review of attachment 152833 [details] [review]:

Looks good, please commit :)
Comment 8 Tim-Philipp Müller 2010-02-16 00:13:35 UTC
Some late night ramblings/questions:

 - aren't sequence headers kind of frequent?

 - if yes, shouldn't we rate-limit the tag posting a bit?

 - and shouldn't we be taking a running average or
   something instead of just posting the latest rate?
Comment 9 Arun Raghavan 2010-02-23 13:05:07 UTC
While the docs suggest that the sequence header could be emitted every 0.6 seconds in the MPEG-2 case, I've not been able to find any videos with >2 sequence headers. Even then, a new header is only processed if it is different from the old header, so this should be good for now. That said, the value 0x3fff is not a legal bitrate - it implies a VBR stream. Updated patch coming up.
Comment 10 Arun Raghavan 2010-02-23 13:06:42 UTC
Created attachment 154490 [details] [review]
Patch to post bitrate from the MPEG sequence header as a tag

Updated patch which accounts for bitrate=0x3ffff
Comment 11 Tim-Philipp Müller 2010-03-22 12:49:34 UTC
Committed with minor changes, thanks:

commit 8144fee47cc311ed3e5f437a9bcd0d96a19b58e8
Author: Arun Raghavan <arun.raghavan@collabora.co.uk>
Date:   Tue Feb 2 15:49:29 2010 +0530

    mpegvideoparse: Parse bitrate and emit as tag
    
    This patch picks up the bitrate for the stream from the MPEG sequence
    header and emits it as a tag on the source pad.
    
    Fixes #599298.

(mostly: move taglist declaration, and use gst_tag_list_new_full())


Leaving open because of the codec field.
Comment 12 Edward Hervey 2013-06-12 07:18:15 UTC
Updating title.

Would be great if pbutils had a way to automatically figure out the TAG type for caps (which it knows internally) and automatically set GST_TAG_VIDEO_CODEC, GST_TAG_AUDIO_CODEC, ....

Right now it expects the caller to know what type of stream it is.
Comment 13 Arnaud Vrac 2013-08-02 18:59:59 UTC
Created attachment 250736 [details] [review]
mpegdemux: send codec tag for each stream

mpegdemux does not send the codecs tags at all, so here is a patch to do it, exactly like tsdemux does it.

Edward, I'm not sure I understand your last comment, doesn't passing NULL to gst_pb_utils_add_codec_description_to_tag_list automatically detect the right tag to send depending on the caps ?
Comment 14 Sebastian Dröge (slomo) 2013-08-12 12:44:20 UTC
Comment on attachment 250736 [details] [review]
mpegdemux: send codec tag for each stream

commit c4140f9c258fcfe3003f20bac8ea50e6c97173c7
Author: Arnaud Vrac <avrac@freebox.fr>
Date:   Fri Aug 2 20:37:30 2013 +0200

    mpegdemux: send codec tag for each stream
Comment 15 Edward Hervey 2013-08-13 14:35:11 UTC
Closing. both mpeg-ts and -ps demuxer push out codec tags