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 363118 - gst_riff_create_video_caps() should also store variant in the caps
gst_riff_create_video_caps() should also store variant in the caps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal enhancement
: 0.10.11
Assigned To: Stefan Sauer (gstreamer, gtkdoc dev)
GStreamer Maintainers
Depends on:
Blocks: 361636 361637
 
 
Reported: 2006-10-18 11:40 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2006-11-14 19:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
adds the variants (5.18 KB, patch)
2006-10-26 12:28 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
adds the variants (now lowercase) (5.18 KB, patch)
2006-10-27 08:46 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
committed Details | Review

Description Stefan Sauer (gstreamer, gtkdoc dev) 2006-10-18 11:40:21 UTC
gst_riff_create_video_caps() as used by the bundles several FourCC tags into one mimetype. This happens for several *263 variants. The resulting type in the caps is "video/x-h263" in this case. It is important to preserve the variant as not all codecs support them all.

For the H263 variants codecs usually decode garbled video if they can reject variants they don't support.

In http://bugzilla.gnome.org/show_bug.cgi?id=361636 (and #361637) there is a proposal to add a variant element (G_TYPE_STRING) to the caps.

Riff-lib could easily used the 'codec_name' string. Unfortunately there are no standard names or types for this.

Alternatively we could also define an enum in gst-plugins-base/gst-libs/video/video.h as using non-standartized strings for this isn't nice.
Comment 1 Ville Syrjala 2006-10-18 14:47:42 UTC
I used the 'variant = (string)...' thing because it was already used by gst-ffmpeg.
Comment 2 Wim Taymans 2006-10-18 16:53:08 UTC
http://gstreamer.freedesktop.org/data/doc/gstreamer/head/pwg/html/section-types-definitions.html

Has an overview of the types in use and their properties. gst-ffmpeg has indeed the variant thing.
I would then go for:

 variant = (string) { "h263p", "intel", ...}


Comment 3 Stefan Sauer (gstreamer, gtkdoc dev) 2006-10-20 07:03:45 UTC
Unfortunately it even more complicated.

The avidemuxer will need to map fourcc to variants and put this into the caps:
variant = { "ITU", "Lead", "Microsoft", "VDOLive", "Vivo", "Xirlink", "Intel", NULL };

then the h263decoders can accept or reject the variant.

On the other hand h263encoders will need to tell followup elements on the bitstream version scheme, so that e.g. payloaders can use the right pt.
h263version = { "h263p", "h236pp" };

The h263version is something different that the variant. the container does not know about it and decoders will need to reject it if the can't handle it, when decoding the first buffer.

@Ville: okay for use to use the term 'h263version' (like in mpegversion and msmpegversion). If so I add the mapping from fourcc to variant to avidemuxer plus document the two new properties in the pwg.
Comment 4 Ville Syrjala 2006-10-23 09:01:08 UTC
So all these variants "ITU", "Lead" etc. are more or less incompatible with each other?
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2006-10-23 14:03:31 UTC
Yes, they are. Its basically vendors that tweaked the bitstreams to save some bits here and there for some special purpose.
Comment 6 Ville Syrjala 2006-10-24 09:02:05 UTC
How do these relate to the H.263 version? Should the version and variant things be separate or is each variant just derived from one version.
Comment 7 Stefan Sauer (gstreamer, gtkdoc dev) 2006-10-24 10:44:18 UTC
Thats why I suggest 'h263version'. This only applies to H263 (the standart format). As far as I know, the other variants don't have versions.
Comment 8 Ville Syrjala 2006-10-24 11:06:50 UTC
In that case h263version isn't really needed. Just having "h263", "h263p" and "h263pp" variants for the standard format should suffice. But either way is fine by me.
Comment 9 Stefan Sauer (gstreamer, gtkdoc dev) 2006-10-24 11:20:50 UTC
Two reasons why I would keep it separate:
* I don't know for sure t the others have versions too.
* The variant is set on the demuxer and checked by a decoder, the ?263version is set by encoder and used by e.g. payloaders

I'll make the change for the avidemuxer real soon.
Comment 10 Ville Syrjala 2006-10-24 11:43:47 UTC
> I don't know for sure t the others have versions too.

OK.

> The variant is set on the demuxer and checked by a decoder, the ?263version
> is set by encoder and used by e.g. payloaders

No, both variant and h263version should be set by demuxers, depayloaders and encoders, and both should be checked by muxers, payloaders and decoders.
Comment 11 Stefan Sauer (gstreamer, gtkdoc dev) 2006-10-24 15:18:53 UTC
IMHO, containers like avi don't cary information about wheter its H263, H263+ or H263++. Looks more like the +/++ streams are using MPEG4 stream extensions. I don't know too much about this, so if you have more info about, please let me know.
Comment 12 Stefan Sauer (gstreamer, gtkdoc dev) 2006-10-26 12:28:38 UTC
Created attachment 75438 [details] [review]
adds the variants

this also adds some missing video template entries
Comment 13 Ville Syrjala 2006-10-26 13:08:10 UTC
Should the variant names be all lowercase? audio/x-adpcm and video/x-rle layout strings are all lowercase.
Comment 14 Stefan Sauer (gstreamer, gtkdoc dev) 2006-10-27 08:46:31 UTC
Created attachment 75495 [details] [review]
adds the variants (now lowercase)

that makes sense, variant names are lowercase
Comment 15 Stefan Sauer (gstreamer, gtkdoc dev) 2006-11-07 07:22:59 UTC
2006-11-07  Stefan Kost  <ensonic@users.sf.net>

	* gst-libs/gst/riff/riff-media.c: (gst_riff_create_video_caps),
	(gst_riff_create_video_template_caps):
	  add h263/h264 variants to the caps, Fixes #363118