GNOME Bugzilla – Bug 782337
jpeg2000parse: parse main level from j2k code stream and add to caps
Last modified: 2017-06-07 07:07:22 UTC
The j2k RSIZ box contains a capabilities tag with information about profile and bit rate. For broadcast streams, the caps contains the profile number and the main level (bit rate) The parser can read the main level and add to caps as property.
The muxer can then read the property and fill in j2k video descriptor. Is there an existing caps field for bit rate ?
It would also be nice to be able to "transcode" a series of non-broadcast images by changing the profile to broadcast + a specified bit rate
Created attachment 351746 [details] [review] Parse JPEG 2000 capabilities tag
I decided to store the complete 16 bit capabilities value in the jpeg2000parse source caps. The mpegts muxer can easily extract the bit rate, and also enforce broadcast profile as specified by standard ( there are several possible J2K profiles, but only broadcast single tile is allowed by standard) .
Need this patch to continue work on #753323 :)
Review of attachment 351746 [details] [review]: ::: gst/videoparsers/gstjpeg2000parse.c @@ +74,3 @@ GST_JPEG2000_SAMPLING_LIST "," + GST_JPEG2000_COLORSPACE_LIST "," + " capabilities = (int)[1, MAX]," " parsed = (boolean) true") Do you *always* add it? And valid is [0, 65535] I guess, not [1, MAX] :) @@ +369,3 @@ + goto beach; + + if (!gst_byte_reader_get_uint16_be (&reader, &capabilities)) What exactly does this integer contain? Would it make sense to split it into multiple fields, or to attach it opaquely as a GstBuffer?
Thanks for the review. (In reply to Sebastian Dröge (slomo) from comment #6) > Review of attachment 351746 [details] [review] [review]: > > ::: gst/videoparsers/gstjpeg2000parse.c > @@ +74,3 @@ > GST_JPEG2000_SAMPLING_LIST "," > + GST_JPEG2000_COLORSPACE_LIST "," > + " capabilities = (int)[1, MAX]," " parsed = (boolean) true") > > Do you *always* add it? And valid is [0, 65535] I guess, not [1, MAX] :) > Yes, all J2K code streams must have this tag, so it makes sense to always add it. And yes, should be [0, 65535] . > @@ +369,3 @@ > + goto beach; > + > + if (!gst_byte_reader_get_uint16_be (&reader, &capabilities)) > > What exactly does this integer contain? Would it make sense to split it into > multiple fields, or to attach it opaquely as a GstBuffer? This integer contains two fields : JPEG 2000 profile (profile 0, profile 1, broadcast, IMF) and main level (a number from 0 to 11 that corresponds to maximum bit rate). The logic for extracting the profile and main level varies depending on what profile we are dealing with. The MPEG TS muxer needs both fields to create a compliant stream.
So let's better have both fields then as separate integers, and also give the appropriate ranges. This way decoders can e.g. specify in their caps that they only handle profile 0 for example.
Ok, good idea. How about "j2k_profile" and "bit_rate" ?
"profile" and "level" or "main-level"? Why call it bitrate if you call it "main level" above? :)
"profile" and "main-level" sounds good. For profile, there are only around 4 or 5 possible values, so I will treat it the same as the sampling field. I will put all of the #defines for profile and main level in the parser header file. Patch coming shortly :)
Note: there are two parts to the JPEG 2000 standard - we will only support profiles for Part 1, as our encoder and decoder don't support Part 2.
Created attachment 351988 [details] [review] Parse profile, main level and sub-level and put in source caps Parse J2K capabilities, and detect profile and main level (broadcast) and sub-level(broadcast and IMF). Do sanity checks on profile, main level and sub level. Put profile in source caps, and optionally put main level and sub level in caps.
Note: main level and sub level are optional - main level is present for broadcast and IMF profiles, and sub level is only present for IMF profile.
Created attachment 351990 [details] [review] Updated comments on last patch
Created attachment 352034 [details] [review] Updated patch Do not reject Part 2 code stream, just because we can't handle it in gstreamer
Created attachment 352052 [details] [review] Final version of patch
Created attachment 352053 [details] One last tweak
Created attachment 352054 [details] Problem with the last patch
Created attachment 352055 [details] [review] Patch wasn't working from windows box, trying Ubuntu
I have a few other patches queued that rely on this patch :) .......
(patches for other issues - this patch is ready to go)
Hi Sebastian, Does this patch look OK? I can't really move forward on the J2K MPEG-TS issue until this one gets resolved. Thanks! Aaron
a very polite <ping/> for this patch :) if anyone has time to take a look.
Review of attachment 352055 [details] [review]: Generally looks good to me ::: gst/videoparsers/gstjpeg2000parse.c @@ +103,3 @@ GST_JPEG2000_SAMPLING_LIST "," + GST_JPEG2000_COLORSPACE_LIST "," + " profile = (int)[0, 49151]," " parsed = (boolean) true") Will we always add the profile in any case, and it's something between those two values (and including them)?
Comment on attachment 352055 [details] [review] Patch wasn't working from windows box, trying Ubuntu Answering my own question, yes. Going to merge later. Thanks!
Great, thanks!
commit 9ec2b7ce131bee726289ca1dda91d09e59775684 Author: Aaron Boxer <boxerab@gmail.com> Date: Fri May 12 15:28:46 2017 -0400 jpeg2000parse: parse RSIZ capabilities and put profile/level into the caps The RSIZ capabilities tag stores the JPEG 2000 profile. In the case of broadcast profiles, it also stores the broadcast main level, which specifies the bit rate. https://bugzilla.gnome.org/show_bug.cgi?id=782337