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 613094 - [flvmux] put more info (like width and height) in the metadata
[flvmux] put more info (like width and height) in the metadata
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal enhancement
: 0.10.22
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-03-17 00:31 UTC by Jan Urbański
Modified: 2010-03-18 09:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
put width and height in metadata (2.51 KB, patch)
2010-03-17 00:32 UTC, Jan Urbański
committed Details | Review
put framerate, codec id and filesize in the metadata (9.01 KB, patch)
2010-03-18 01:05 UTC, Jan Urbański
committed Details | Review

Description Jan Urbański 2010-03-17 00:31:45 UTC
Players use the width and height info to scale the display if PAR is not provided or if additional scaling is to be avoided.

The bug's summary is more general, because there's other data that might be interesting for the players. For instance, common FLV metadata injectors put:

 * filesize in bytes
 * last timestamp
 * framerate
 * number of video and audio tags
 * audio and video codec IDs
 * canSeekToEnd info (if the last video tag is a keyframe)
 * ... other crap

in the metadata. See for instance http://www.buraks.com/flvmdi/.

I'm not sure how much of that info is actually used by anything out there, but since we have the info, maybe it's worth it to put in in there?
Comment 1 Jan Urbański 2010-03-17 00:32:28 UTC
Created attachment 156316 [details] [review]
put width and height in metadata
Comment 2 Sebastian Dröge (slomo) 2010-03-17 08:28:29 UTC
commit dcb5afd351b6ad89abf93e22a43a421a1722c489
Author: Jan Urbański <wulczer@wulczer.org>
Date:   Tue Mar 16 23:43:39 2010 +0100

    flvmux: Put width and height in the metadata
    
    Some players use that info to scale their display.
    
    See #613094.
Comment 3 Sebastian Dröge (slomo) 2010-03-17 08:29:55 UTC
Review of attachment 156316 [details] [review]:

::: gst/flv/gstflvmux.c
@@ +789,3 @@
         d = par_x;
+        GST_DEBUG_OBJECT (mux, "putting AspectRatioX %f in the metadata", d);
+

Should we put the PAR or the DAR inside the metadata? :) I know you didn't touch that code but I just noticed this while looking at your changes
Comment 4 Jan Urbański 2010-03-17 09:02:10 UTC
The code to put the PAR in metadata was already there, I just added debugging ;)

Putting the PAR has a lot of sense, because it allows players to scale the display accordingly (and using the Flash APIs you can't get the PAR from the actual encoded video data, so you need to rely on the textual stuff in the metadata). The DAR doesn't make sense, right? Because you'd put in the DAR of the machine on which you are encoding... And Flash can get the DAR of the display that's playing the content, so that's not really necessary.

I haven't seen a player that makes use of other things than duration, PAR info and width/height, but the filesize, for instance, might be useful for a "Download this file" button, canSeekToEnd for a "seek to end" button, codec IDs to tell the client that he's listening to Speex etc.

I think filesize, maybe the framerate and maybe codec info would be useful (filesize in non-live case only).
Comment 5 Sebastian Dröge (slomo) 2010-03-17 09:33:33 UTC
(In reply to comment #4)

> Putting the PAR has a lot of sense, because it allows players to scale the
> display accordingly (and using the Flash APIs you can't get the PAR from the
> actual encoded video data, so you need to rely on the textual stuff in the
> metadata). The DAR doesn't make sense, right? Because you'd put in the DAR of
> the machine on which you are encoding... And Flash can get the DAR of the
> display that's playing the content, so that's not really necessary.

Well, the PAR describes the width/height ratio of a pixel while the DAR describes the same for a complete frame. Typical DARs are 16:9, 4:3, etc.

Together with the width/height you can convert from PAR to DAR and back.

> I think filesize, maybe the framerate and maybe codec info would be useful
> (filesize in non-live case only).

Yes, these might indeed be useful to add.
Comment 6 Jan Urbański 2010-03-17 09:38:25 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > The DAR doesn't make sense, right? Because you'd put in the DAR of
> > the machine on which you are encoding... And Flash can get the DAR of the
> > display that's playing the content, so that's not really necessary.
> 
> Well, the PAR describes the width/height ratio of a pixel while the DAR
> describes the same for a complete frame. Typical DARs are 16:9, 4:3, etc.
> 
> Together with the width/height you can convert from PAR to DAR and back.

OK, so I mixing things up. If you can convert them back and forth, I think width and height + PAR is enough, and I've never seen any FLV file with DAR info in the metadata.

> > I think filesize, maybe the framerate and maybe codec info would be useful
> > (filesize in non-live case only).
> 
> Yes, these might indeed be useful to add.

OK, patch coming in the evening.
Comment 7 Jan Urbański 2010-03-18 01:05:27 UTC
Created attachment 156423 [details] [review]
put framerate, codec id and filesize in the metadata
Comment 8 Sebastian Dröge (slomo) 2010-03-18 09:02:22 UTC
commit 7d32f46b7a8f31955262c502a927ae3e06780149
Author: Jan Urbański <wulczer@wulczer.org>
Date:   Thu Mar 18 01:51:19 2010 +0100

    flvmux: put more information in the metadata
    
    Additional tags are: audiocodecid, videocodecid framerate and (in the
    non-live case) filesize.
    
    While at it, fix index rewriting to update duration and filesize
    values even if the index is empty.
    
    Fixes #613094.