[vlc-devel] [PATCH 2/5] hxxx_sei: add HDR10/SMPTE ST 2086 metadata parsing

Filip Roséen filip at atch.se
Wed Mar 29 18:59:35 CEST 2017


Hi Steve and Francois,

On 2017-03-29 17:02, Steve Lhomme wrote:

> On Wed, Mar 29, 2017 at 4:56 PM, Filip Roséen <filip at atch.se> wrote:
> > Hi Steve,
> >
> > On 2017-03-29 15:39, Steve Lhomme wrote:
> >
> >  ---
> >   modules/packetizer/hxxx_sei.c | 14 ++++++++++++++
> >   modules/packetizer/hxxx_sei.h | 11 ++++++++++-
> >   2 files changed, 24 insertions(+), 1 deletion(-)
> >
> >  diff --git a/modules/packetizer/hxxx_sei.c b/modules/packetizer/hxxx_sei.c
> >  index e83528cb7c..636c7178cf 100644
> >  --- a/modules/packetizer/hxxx_sei.c
> >  +++ b/modules/packetizer/hxxx_sei.c
> >  @@ -145,6 +145,20 @@ void HxxxParseSEI(const uint8_t *p_buf, size_t i_buf,
> >                   b_continue = pf_callback( &sei_data, cbdata );
> >               } break;
> >
> >  +            case HXXX_SEI_MASTERING_DISPLAY_COLOUR_VOLUME:
> >  +            {
> >  +                if ( bs_remain( &s ) < (16*4*2+32+32))
> >  +                    /* not enough data */
> >
> > The above check does not seem to match the reading that takes place in this
> > block, as the actual reading potentially consumes 16*6 + 16*4 + 32 + 32 =>
> > 224, wheras the current check uses 192.
> 
> Nope, the second is 16*2, not 16*4.

Oh yeah right, I completely misread the loop conditions and/or
calculations for some reason; sorry about that.

Though I must say that the expression used to calculate the minimum
number of bits is rather odd. The expression `16*4*2` is course is
equivalent to `16*8` (where `8` denotes the total number of 16-bit
reads), but it implies code logic that does not reflect what is
coming.

In my opinion, especially given the brainfart the expression caused in
my brain, it would be far easier to reason about (and maintain) `16*(4
+ 2) + 32 + 32`, or the more verbose (but more in-line with the
implementation) `16*4 + 16*2 + 32 + 32`.

The above is nitpicking in its most basic form, and I guess I am
just trying to salvage whatever insanity it might have (or might not
have) left.

Best Regards,\
Filip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170329/f1b36c20/attachment.html>


More information about the vlc-devel mailing list