Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-02-13 Thread Michael Niedermayer 
On Mon, Feb 08, 2016 at 09:36:42AM -0800, Neil Birkbeck wrote:
> I'm sending an updated patch that persists the data within the CVS
> (between IRAP access units with no_rasl_output_flag=1). If this seems
> like overkill, we can fallback to Hendrik's suggestion of just sending
> it once. I did consider this alternative but figured the side data
> could get lost if some frames were dropped in some part of the filter
> chain.
> 
> 
> On Thu, Feb 4, 2016 at 11:04 PM, Hendrik Leppkes  wrote:
> > On Fri, Feb 5, 2016 at 3:05 AM, Neil Birkbeck  
> > wrote:
> >> According to the ITU-T H.265 v3, in Table D - 1, the persistence scope
> >> of the mastering display metadata is "The Coded Video Sequence (CVS)
> >> containing the SEI message". So I guess we want to clear when we start
> >> the next CVS, so I guess when we see the next IDR frame.
> >>
> >> Given that the SEI comes before the IDR, I don't see a great way to do
> >> this aside from initializing the flag to 2 and decrementing (when > 0)
> >> when we see an IDR.
> >>
> >
> > The usual way I would handle such SEI metadata is just to send the
> > sidedata once when it appears in the bitstream and not repeat it with
> > every single frame, and let the user worry about its lifetime.
> >
> > - Hendrik
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

>  hevc.c |   52 
>  hevc.h |7 +++
>  hevc_sei.c |   26 ++
>  version.h  |2 +-
>  4 files changed, 86 insertions(+), 1 deletion(-)
> 6b6743db49b2982e684f6ef2df94d336bae76947  
> 0001-lavc-hevc-Parse-SEI_TYPE_MASTERING_DISPLAY_INFO-and-.patch
> From e6602d1191a9166e5a38c4c27b430fbe0b84aeca Mon Sep 17 00:00:00 2001
> From: Neil Birkbeck 
> Date: Thu, 21 Jan 2016 10:56:50 -0800
> Subject: [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate
>  content into the AVMasteringDisplayMetadata side data.
> 
> Add support for parsing SEI_TYPE_MASTERING_DISPLAY_INFO and propagate 
> contents into
> the AVMasteringDisplayMetadata side data. Primaries are ordered in RGB order 
> and
> the values are converted to rationals ([0,1] for CEI 1931 Chroma coords,
> and cd/m^2 for luma).
> 
> Signed-off-by: Neil Birkbeck 
> ---
>  libavcodec/hevc.c | 52 
> +++
>  libavcodec/hevc.h |  7 +++
>  libavcodec/hevc_sei.c | 26 ++
>  libavcodec/version.h  |  2 +-
>  4 files changed, 86 insertions(+), 1 deletion(-)

applied

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-02-08 Thread Neil Birkbeck 
I'm sending an updated patch that persists the data within the CVS
(between IRAP access units with no_rasl_output_flag=1). If this seems
like overkill, we can fallback to Hendrik's suggestion of just sending
it once. I did consider this alternative but figured the side data
could get lost if some frames were dropped in some part of the filter
chain.


On Thu, Feb 4, 2016 at 11:04 PM, Hendrik Leppkes  wrote:
> On Fri, Feb 5, 2016 at 3:05 AM, Neil Birkbeck  wrote:
>> According to the ITU-T H.265 v3, in Table D - 1, the persistence scope
>> of the mastering display metadata is "The Coded Video Sequence (CVS)
>> containing the SEI message". So I guess we want to clear when we start
>> the next CVS, so I guess when we see the next IDR frame.
>>
>> Given that the SEI comes before the IDR, I don't see a great way to do
>> this aside from initializing the flag to 2 and decrementing (when > 0)
>> when we see an IDR.
>>
>
> The usual way I would handle such SEI metadata is just to send the
> sidedata once when it appears in the bitstream and not repeat it with
> every single frame, and let the user worry about its lifetime.
>
> - Hendrik
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


0001-lavc-hevc-Parse-SEI_TYPE_MASTERING_DISPLAY_INFO-and-.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-02-04 Thread Hendrik Leppkes 
On Fri, Feb 5, 2016 at 3:05 AM, Neil Birkbeck  wrote:
> According to the ITU-T H.265 v3, in Table D - 1, the persistence scope
> of the mastering display metadata is "The Coded Video Sequence (CVS)
> containing the SEI message". So I guess we want to clear when we start
> the next CVS, so I guess when we see the next IDR frame.
>
> Given that the SEI comes before the IDR, I don't see a great way to do
> this aside from initializing the flag to 2 and decrementing (when > 0)
> when we see an IDR.
>

The usual way I would handle such SEI metadata is just to send the
sidedata once when it appears in the bitstream and not repeat it with
every single frame, and let the user worry about its lifetime.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-02-04 Thread Neil Birkbeck 
According to the ITU-T H.265 v3, in Table D - 1, the persistence scope
of the mastering display metadata is "The Coded Video Sequence (CVS)
containing the SEI message". So I guess we want to clear when we start
the next CVS, so I guess when we see the next IDR frame.

Given that the SEI comes before the IDR, I don't see a great way to do
this aside from initializing the flag to 2 and decrementing (when > 0)
when we see an IDR.

On Tue, Feb 2, 2016 at 7:13 AM, Michael Niedermayer
 wrote:
> On Mon, Feb 01, 2016 at 12:11:10PM -0800, Neil Birkbeck wrote:
>> Please see updated patch.
>>
>> On Mon, Jan 25, 2016 at 11:39 PM, Neil Birkbeck  
>> wrote:
>> > I guess png is another example; 10 is the denominator for the 32-bit
>> > integer:
>> > https://github.com/FFmpeg/FFmpeg/blob/b58cfa616c169c90166938608e7135cdab5820e0/libavcodec/pngenc.c#L290
>> >
>> > In the standards, the primaries and white point coords are usually only
>> > referenced in 2-4 significant figures (ISO/IEC 23001-8:2013(E) has them all
>> > in one doc). The display primaries can be different from these, but I don't
>> > think you'd see them measured with any more precision (more precision is
>> > unlikely to matter for this metadata anyway).
>> >
>> > It seems that most are in favor of AVRational, so I'll change the avutil
>> > struct float internal fields to rational.
>> >
>> >
>> >
>> >
>> >
>> > On Mon, Jan 25, 2016 at 1:43 PM, Hendrik Leppkes 
>> > wrote:
>> >>
>> >> On Mon, Jan 25, 2016 at 10:37 PM, Michael Niedermayer
>> >>  wrote:
>> >> > On Fri, Jan 22, 2016 at 02:54:21PM -0800, Neil Birkbeck wrote:
>> >> >> Hmm. I don't have a good idea of how likely it is for this conversion
>> >> >> to
>> >> >> float (by dividing a constant) to not be bit-exact on different
>> >> >> architectures (compilers?) when there should not be any other math
>> >> >> transforming the metadata (other than the conversion back to the
>> >> >> integer
>> >> >> coding for cases like hevc, which for a given architecture is possible
>> >> >> without loss). The fact that this could happen at all does make it
>> >> >> annoying
>> >> >> in terms of bit-exact test expectations across arch, and this is the
>> >> >> main
>> >> >> concern, right? (for this type of metadata, it is really a hint to
>> >> >> TVs/algorithms, and some will ignore it altogether)
>> >> >
>> >> > bitexactness is one concern, also theres the issue with what is ideally
>> >> > correct.
>> >> > that is what are the ideal values dictated by various standards
>> >> > that hardware (cammeras, ...) aim at ?
>> >> > are these rational or float or what can represent them better ?
>> >> >
>> >>
>> >> Both HEVC and the HDMI Info Frames use fixed-point integers (the same
>> >> scales too, apparently), I do not know of the formats anything else
>> >> uses.
>> >> Maybe we should be using AVRationals?
>> >>
>> >> I would argue that MKV storing floats is a terrible idea, and someone
>> >> should bonk them over the head and store fixed point as well.
>> >>
>> >> - Hendrik
>> >> ___
>> >> ffmpeg-devel mailing list
>> >> ffmpeg-devel@ffmpeg.org
>> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >
>> >
>
>>  hevc.c |   46 ++
>>  hevc.h |7 +++
>>  hevc_sei.c |   22 ++
>>  version.h  |2 +-
>>  4 files changed, 76 insertions(+), 1 deletion(-)
>> a1660d647add89f41e8ed85f8bc26eb4fb36379b  
>> 0001-lavc-hevc-Parse-SEI_TYPE_MASTERING_DISPLAY_INFO-and-.patch
>> From 345db2caf7cac5c0acfb74501b6db9bd57e66f7d Mon Sep 17 00:00:00 2001
>> From: Neil Birkbeck 
>> Date: Thu, 21 Jan 2016 10:56:50 -0800
>> Subject: [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and 
>> propagate
>>  content into the AVMasteringDisplayMetadata side data.
>>
>> Add support for parsing SEI_TYPE_MASTERING_DISPLAY_INFO and propagate 
>> contents into
>> the AVMasteringDisplayMetadata side data. Primaries are ordered in RGB order 
>> and
>> the values are converted to rationals ([0,1] for CEI 1931 Chroma coords,
>> and cd/m^2 for luma).
>>
>> Signed-off-by: Neil Birkbeck 
>> ---
>>  libavcodec/hevc.c | 46 ++
>>  libavcodec/hevc.h |  7 +++
>>  libavcodec/hevc_sei.c | 22 ++
>>  libavcodec/version.h  |  2 +-
>>  4 files changed, 76 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
>> index c245d3b..e475f94 100644
>> --- a/libavcodec/hevc.c
>> +++ b/libavcodec/hevc.c
>> @@ -28,6 +28,7 @@
>>  #include "libavutil/common.h"
>>  #include "libavutil/display.h"
>>  #include "libavutil/internal.h"
>> +#include "libavutil/mastering_display_metadata.h"
>>  #include "libavutil/md5.h"
>>  #include "libavutil/opt.h"
>>  #include "libavutil/pixdesc.h"
>> @@ -2580,6 +2581,51 @@

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-02-04 Thread Michael Niedermayer 
On Thu, Feb 04, 2016 at 06:05:08PM -0800, Neil Birkbeck wrote:
> According to the ITU-T H.265 v3, in Table D - 1, the persistence scope
> of the mastering display metadata is "The Coded Video Sequence (CVS)
> containing the SEI message". So I guess we want to clear when we start
> the next CVS, so I guess when we see the next IDR frame.
> 

> Given that the SEI comes before the IDR, I don't see a great way to do
> this aside from initializing the flag to 2 and decrementing (when > 0)
> when we see an IDR.

hmm, yes, i dont see a nicer way atm either


> 
> On Tue, Feb 2, 2016 at 7:13 AM, Michael Niedermayer
>  wrote:
> > On Mon, Feb 01, 2016 at 12:11:10PM -0800, Neil Birkbeck wrote:
> >> Please see updated patch.
> >>
> >> On Mon, Jan 25, 2016 at 11:39 PM, Neil Birkbeck  
> >> wrote:
> >> > I guess png is another example; 10 is the denominator for the 32-bit
> >> > integer:
> >> > https://github.com/FFmpeg/FFmpeg/blob/b58cfa616c169c90166938608e7135cdab5820e0/libavcodec/pngenc.c#L290
> >> >
> >> > In the standards, the primaries and white point coords are usually only
> >> > referenced in 2-4 significant figures (ISO/IEC 23001-8:2013(E) has them 
> >> > all
> >> > in one doc). The display primaries can be different from these, but I 
> >> > don't
> >> > think you'd see them measured with any more precision (more precision is
> >> > unlikely to matter for this metadata anyway).
> >> >
> >> > It seems that most are in favor of AVRational, so I'll change the avutil
> >> > struct float internal fields to rational.
> >> >
> >> >
> >> >
> >> >
> >> >
> >> > On Mon, Jan 25, 2016 at 1:43 PM, Hendrik Leppkes 
> >> > wrote:
> >> >>
> >> >> On Mon, Jan 25, 2016 at 10:37 PM, Michael Niedermayer
> >> >>  wrote:
> >> >> > On Fri, Jan 22, 2016 at 02:54:21PM -0800, Neil Birkbeck wrote:
> >> >> >> Hmm. I don't have a good idea of how likely it is for this conversion
> >> >> >> to
> >> >> >> float (by dividing a constant) to not be bit-exact on different
> >> >> >> architectures (compilers?) when there should not be any other math
> >> >> >> transforming the metadata (other than the conversion back to the
> >> >> >> integer
> >> >> >> coding for cases like hevc, which for a given architecture is 
> >> >> >> possible
> >> >> >> without loss). The fact that this could happen at all does make it
> >> >> >> annoying
> >> >> >> in terms of bit-exact test expectations across arch, and this is the
> >> >> >> main
> >> >> >> concern, right? (for this type of metadata, it is really a hint to
> >> >> >> TVs/algorithms, and some will ignore it altogether)
> >> >> >
> >> >> > bitexactness is one concern, also theres the issue with what is 
> >> >> > ideally
> >> >> > correct.
> >> >> > that is what are the ideal values dictated by various standards
> >> >> > that hardware (cammeras, ...) aim at ?
> >> >> > are these rational or float or what can represent them better ?
> >> >> >
> >> >>
> >> >> Both HEVC and the HDMI Info Frames use fixed-point integers (the same
> >> >> scales too, apparently), I do not know of the formats anything else
> >> >> uses.
> >> >> Maybe we should be using AVRationals?
> >> >>
> >> >> I would argue that MKV storing floats is a terrible idea, and someone
> >> >> should bonk them over the head and store fixed point as well.
> >> >>
> >> >> - Hendrik
> >> >> ___
> >> >> ffmpeg-devel mailing list
> >> >> ffmpeg-devel@ffmpeg.org
> >> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >
> >> >
> >
> >>  hevc.c |   46 ++
> >>  hevc.h |7 +++
> >>  hevc_sei.c |   22 ++
> >>  version.h  |2 +-
> >>  4 files changed, 76 insertions(+), 1 deletion(-)
> >> a1660d647add89f41e8ed85f8bc26eb4fb36379b  
> >> 0001-lavc-hevc-Parse-SEI_TYPE_MASTERING_DISPLAY_INFO-and-.patch
> >> From 345db2caf7cac5c0acfb74501b6db9bd57e66f7d Mon Sep 17 00:00:00 2001
> >> From: Neil Birkbeck 
> >> Date: Thu, 21 Jan 2016 10:56:50 -0800
> >> Subject: [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and 
> >> propagate
> >>  content into the AVMasteringDisplayMetadata side data.
> >>
> >> Add support for parsing SEI_TYPE_MASTERING_DISPLAY_INFO and propagate 
> >> contents into
> >> the AVMasteringDisplayMetadata side data. Primaries are ordered in RGB 
> >> order and
> >> the values are converted to rationals ([0,1] for CEI 1931 Chroma coords,
> >> and cd/m^2 for luma).
> >>
> >> Signed-off-by: Neil Birkbeck 
> >> ---
> >>  libavcodec/hevc.c | 46 ++
> >>  libavcodec/hevc.h |  7 +++
> >>  libavcodec/hevc_sei.c | 22 ++
> >>  libavcodec/version.h  |  2 +-
> >>  4 files changed, 76 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
> >> index c245d3b..e475f94

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-02-02 Thread Michael Niedermayer 
On Mon, Feb 01, 2016 at 12:11:10PM -0800, Neil Birkbeck wrote:
> Please see updated patch.
> 
> On Mon, Jan 25, 2016 at 11:39 PM, Neil Birkbeck  
> wrote:
> > I guess png is another example; 10 is the denominator for the 32-bit
> > integer:
> > https://github.com/FFmpeg/FFmpeg/blob/b58cfa616c169c90166938608e7135cdab5820e0/libavcodec/pngenc.c#L290
> >
> > In the standards, the primaries and white point coords are usually only
> > referenced in 2-4 significant figures (ISO/IEC 23001-8:2013(E) has them all
> > in one doc). The display primaries can be different from these, but I don't
> > think you'd see them measured with any more precision (more precision is
> > unlikely to matter for this metadata anyway).
> >
> > It seems that most are in favor of AVRational, so I'll change the avutil
> > struct float internal fields to rational.
> >
> >
> >
> >
> >
> > On Mon, Jan 25, 2016 at 1:43 PM, Hendrik Leppkes 
> > wrote:
> >>
> >> On Mon, Jan 25, 2016 at 10:37 PM, Michael Niedermayer
> >>  wrote:
> >> > On Fri, Jan 22, 2016 at 02:54:21PM -0800, Neil Birkbeck wrote:
> >> >> Hmm. I don't have a good idea of how likely it is for this conversion
> >> >> to
> >> >> float (by dividing a constant) to not be bit-exact on different
> >> >> architectures (compilers?) when there should not be any other math
> >> >> transforming the metadata (other than the conversion back to the
> >> >> integer
> >> >> coding for cases like hevc, which for a given architecture is possible
> >> >> without loss). The fact that this could happen at all does make it
> >> >> annoying
> >> >> in terms of bit-exact test expectations across arch, and this is the
> >> >> main
> >> >> concern, right? (for this type of metadata, it is really a hint to
> >> >> TVs/algorithms, and some will ignore it altogether)
> >> >
> >> > bitexactness is one concern, also theres the issue with what is ideally
> >> > correct.
> >> > that is what are the ideal values dictated by various standards
> >> > that hardware (cammeras, ...) aim at ?
> >> > are these rational or float or what can represent them better ?
> >> >
> >>
> >> Both HEVC and the HDMI Info Frames use fixed-point integers (the same
> >> scales too, apparently), I do not know of the formats anything else
> >> uses.
> >> Maybe we should be using AVRationals?
> >>
> >> I would argue that MKV storing floats is a terrible idea, and someone
> >> should bonk them over the head and store fixed point as well.
> >>
> >> - Hendrik
> >> ___
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> >

>  hevc.c |   46 ++
>  hevc.h |7 +++
>  hevc_sei.c |   22 ++
>  version.h  |2 +-
>  4 files changed, 76 insertions(+), 1 deletion(-)
> a1660d647add89f41e8ed85f8bc26eb4fb36379b  
> 0001-lavc-hevc-Parse-SEI_TYPE_MASTERING_DISPLAY_INFO-and-.patch
> From 345db2caf7cac5c0acfb74501b6db9bd57e66f7d Mon Sep 17 00:00:00 2001
> From: Neil Birkbeck 
> Date: Thu, 21 Jan 2016 10:56:50 -0800
> Subject: [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate
>  content into the AVMasteringDisplayMetadata side data.
> 
> Add support for parsing SEI_TYPE_MASTERING_DISPLAY_INFO and propagate 
> contents into
> the AVMasteringDisplayMetadata side data. Primaries are ordered in RGB order 
> and
> the values are converted to rationals ([0,1] for CEI 1931 Chroma coords,
> and cd/m^2 for luma).
> 
> Signed-off-by: Neil Birkbeck 
> ---
>  libavcodec/hevc.c | 46 ++
>  libavcodec/hevc.h |  7 +++
>  libavcodec/hevc_sei.c | 22 ++
>  libavcodec/version.h  |  2 +-
>  4 files changed, 76 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
> index c245d3b..e475f94 100644
> --- a/libavcodec/hevc.c
> +++ b/libavcodec/hevc.c
> @@ -28,6 +28,7 @@
>  #include "libavutil/common.h"
>  #include "libavutil/display.h"
>  #include "libavutil/internal.h"
> +#include "libavutil/mastering_display_metadata.h"
>  #include "libavutil/md5.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
> @@ -2580,6 +2581,51 @@ static int set_side_data(HEVCContext *s)
> s->sei_hflip, s->sei_vflip);
>  }
>  
> +if (s->sei_mastering_display_info_present) {
> +// HEVC uses a g,b,r ordering, which we convert to a more natural 
> r,g,b
> +const int mapping[3] = {2, 0, 1};
> +const int chroma_den = 5;
> +const int luma_den = 1;
> +int i;
> +AVMasteringDisplayMetadata *metadata =
> +av_mastering_display_metadata_create_side_data(out);
> +if (!metadata)
> +return AVERROR(ENOMEM);
> +
> +for (i = 0; i < 3; i++) {
>

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-02-01 Thread Neil Birkbeck 
Please see updated patch.

On Mon, Jan 25, 2016 at 11:39 PM, Neil Birkbeck  wrote:
> I guess png is another example; 10 is the denominator for the 32-bit
> integer:
> https://github.com/FFmpeg/FFmpeg/blob/b58cfa616c169c90166938608e7135cdab5820e0/libavcodec/pngenc.c#L290
>
> In the standards, the primaries and white point coords are usually only
> referenced in 2-4 significant figures (ISO/IEC 23001-8:2013(E) has them all
> in one doc). The display primaries can be different from these, but I don't
> think you'd see them measured with any more precision (more precision is
> unlikely to matter for this metadata anyway).
>
> It seems that most are in favor of AVRational, so I'll change the avutil
> struct float internal fields to rational.
>
>
>
>
>
> On Mon, Jan 25, 2016 at 1:43 PM, Hendrik Leppkes 
> wrote:
>>
>> On Mon, Jan 25, 2016 at 10:37 PM, Michael Niedermayer
>>  wrote:
>> > On Fri, Jan 22, 2016 at 02:54:21PM -0800, Neil Birkbeck wrote:
>> >> Hmm. I don't have a good idea of how likely it is for this conversion
>> >> to
>> >> float (by dividing a constant) to not be bit-exact on different
>> >> architectures (compilers?) when there should not be any other math
>> >> transforming the metadata (other than the conversion back to the
>> >> integer
>> >> coding for cases like hevc, which for a given architecture is possible
>> >> without loss). The fact that this could happen at all does make it
>> >> annoying
>> >> in terms of bit-exact test expectations across arch, and this is the
>> >> main
>> >> concern, right? (for this type of metadata, it is really a hint to
>> >> TVs/algorithms, and some will ignore it altogether)
>> >
>> > bitexactness is one concern, also theres the issue with what is ideally
>> > correct.
>> > that is what are the ideal values dictated by various standards
>> > that hardware (cammeras, ...) aim at ?
>> > are these rational or float or what can represent them better ?
>> >
>>
>> Both HEVC and the HDMI Info Frames use fixed-point integers (the same
>> scales too, apparently), I do not know of the formats anything else
>> uses.
>> Maybe we should be using AVRationals?
>>
>> I would argue that MKV storing floats is a terrible idea, and someone
>> should bonk them over the head and store fixed point as well.
>>
>> - Hendrik
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
From 345db2caf7cac5c0acfb74501b6db9bd57e66f7d Mon Sep 17 00:00:00 2001
From: Neil Birkbeck 
Date: Thu, 21 Jan 2016 10:56:50 -0800
Subject: [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate
 content into the AVMasteringDisplayMetadata side data.

Add support for parsing SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into
the AVMasteringDisplayMetadata side data. Primaries are ordered in RGB order and
the values are converted to rationals ([0,1] for CEI 1931 Chroma coords,
and cd/m^2 for luma).

Signed-off-by: Neil Birkbeck 
---
 libavcodec/hevc.c | 46 ++
 libavcodec/hevc.h |  7 +++
 libavcodec/hevc_sei.c | 22 ++
 libavcodec/version.h  |  2 +-
 4 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
index c245d3b..e475f94 100644
--- a/libavcodec/hevc.c
+++ b/libavcodec/hevc.c
@@ -28,6 +28,7 @@
 #include "libavutil/common.h"
 #include "libavutil/display.h"
 #include "libavutil/internal.h"
+#include "libavutil/mastering_display_metadata.h"
 #include "libavutil/md5.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
@@ -2580,6 +2581,51 @@ static int set_side_data(HEVCContext *s)
s->sei_hflip, s->sei_vflip);
 }
 
+if (s->sei_mastering_display_info_present) {
+// HEVC uses a g,b,r ordering, which we convert to a more natural r,g,b
+const int mapping[3] = {2, 0, 1};
+const int chroma_den = 5;
+const int luma_den = 1;
+int i;
+AVMasteringDisplayMetadata *metadata =
+av_mastering_display_metadata_create_side_data(out);
+if (!metadata)
+return AVERROR(ENOMEM);
+
+for (i = 0; i < 3; i++) {
+const int j = mapping[i];
+metadata->display_primaries[i][0].num = s->display_primaries[j][0];
+metadata->display_primaries[i][0].den = chroma_den;
+metadata->display_primaries[i][1].num = s->display_primaries[j][1];
+metadata->display_primaries[i][1].den = chroma_den;
+}
+metadata->white_point[0].num = s->white_point[0];
+metadata->white_point[0].den = chroma_den;
+metadata->white_point[1].num = s->white_point[1];
+metadata->white_point[1].den = chroma_den;
+
+metadata->max_luminance.num =

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-01-25 Thread Neil Birkbeck 
I guess png is another example; 10 is the denominator for the 32-bit
integer:
https://github.com/FFmpeg/FFmpeg/blob/b58cfa616c169c90166938608e7135cdab5820e0/libavcodec/pngenc.c#L290

In the standards, the primaries and white point coords are usually only
referenced in 2-4 significant figures (ISO/IEC 23001-8:2013(E) has them all
in one doc). The display primaries can be different from these, but I don't
think you'd see them measured with any more precision (more precision is
unlikely to matter for this metadata anyway).

It seems that most are in favor of AVRational, so I'll change the avutil
struct float internal fields to rational.





On Mon, Jan 25, 2016 at 1:43 PM, Hendrik Leppkes 
wrote:

> On Mon, Jan 25, 2016 at 10:37 PM, Michael Niedermayer
>  wrote:
> > On Fri, Jan 22, 2016 at 02:54:21PM -0800, Neil Birkbeck wrote:
> >> Hmm. I don't have a good idea of how likely it is for this conversion to
> >> float (by dividing a constant) to not be bit-exact on different
> >> architectures (compilers?) when there should not be any other math
> >> transforming the metadata (other than the conversion back to the integer
> >> coding for cases like hevc, which for a given architecture is possible
> >> without loss). The fact that this could happen at all does make it
> annoying
> >> in terms of bit-exact test expectations across arch, and this is the
> main
> >> concern, right? (for this type of metadata, it is really a hint to
> >> TVs/algorithms, and some will ignore it altogether)
> >
> > bitexactness is one concern, also theres the issue with what is ideally
> > correct.
> > that is what are the ideal values dictated by various standards
> > that hardware (cammeras, ...) aim at ?
> > are these rational or float or what can represent them better ?
> >
>
> Both HEVC and the HDMI Info Frames use fixed-point integers (the same
> scales too, apparently), I do not know of the formats anything else
> uses.
> Maybe we should be using AVRationals?
>
> I would argue that MKV storing floats is a terrible idea, and someone
> should bonk them over the head and store fixed point as well.
>
> - Hendrik
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-01-25 Thread Hendrik Leppkes 
On Mon, Jan 25, 2016 at 10:37 PM, Michael Niedermayer
 wrote:
> On Fri, Jan 22, 2016 at 02:54:21PM -0800, Neil Birkbeck wrote:
>> Hmm. I don't have a good idea of how likely it is for this conversion to
>> float (by dividing a constant) to not be bit-exact on different
>> architectures (compilers?) when there should not be any other math
>> transforming the metadata (other than the conversion back to the integer
>> coding for cases like hevc, which for a given architecture is possible
>> without loss). The fact that this could happen at all does make it annoying
>> in terms of bit-exact test expectations across arch, and this is the main
>> concern, right? (for this type of metadata, it is really a hint to
>> TVs/algorithms, and some will ignore it altogether)
>
> bitexactness is one concern, also theres the issue with what is ideally
> correct.
> that is what are the ideal values dictated by various standards
> that hardware (cammeras, ...) aim at ?
> are these rational or float or what can represent them better ?
>

Both HEVC and the HDMI Info Frames use fixed-point integers (the same
scales too, apparently), I do not know of the formats anything else
uses.
Maybe we should be using AVRationals?

I would argue that MKV storing floats is a terrible idea, and someone
should bonk them over the head and store fixed point as well.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-01-22 Thread wm4 
On Thu, 21 Jan 2016 15:57:38 -0800
Neil Birkbeck  wrote:

> Thanks for the quick review, Michael.
> 
> The display primaries are encoded as integers (rational with fixed
> denominator, I guess) in h265 and HDMI InfoFrames (but in mkv extensions,
> they will be floats). Float is sufficient to represent the 16-bit integer

Is there still a chance to stop Matroska from making this mistake?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-01-22 Thread Neil Birkbeck 
Also, the mkv adjustments are here:
https://mailarchive.ietf.org/arch/search/?email_list=cellar=1=sZyfPTM-QY69P-0omfOIiTN622o


On Fri, Jan 22, 2016 at 2:54 PM, Neil Birkbeck 
wrote:

> Hmm. I don't have a good idea of how likely it is for this conversion to
> float (by dividing a constant) to not be bit-exact on different
> architectures (compilers?) when there should not be any other math
> transforming the metadata (other than the conversion back to the integer
> coding for cases like hevc, which for a given architecture is possible
> without loss). The fact that this could happen at all does make it annoying
> in terms of bit-exact test expectations across arch, and this is the main
> concern, right? (for this type of metadata, it is really a hint to
> TVs/algorithms, and some will ignore it altogether)
>
> I suppose we already have that problem when converting from some internal
> rational to any float field in mkv (say "Duration", or
> "SamplingFrequency"), as even converting these fields to a AVRational
> inside ffmpeg eventually has to do the division to get a float/double for
> mkv.
>
>
>
>
> On Fri, Jan 22, 2016 at 1:01 AM, wm4  wrote:
>
>> On Thu, 21 Jan 2016 15:57:38 -0800
>> Neil Birkbeck  wrote:
>>
>> > Thanks for the quick review, Michael.
>> >
>> > The display primaries are encoded as integers (rational with fixed
>> > denominator, I guess) in h265 and HDMI InfoFrames (but in mkv
>> extensions,
>> > they will be floats). Float is sufficient to represent the 16-bit
>> integer
>>
>> Is there still a chance to stop Matroska from making this mistake?
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-01-21 Thread Michael Niedermayer 
On Thu, Jan 21, 2016 at 02:26:39PM -0800, Neil Birkbeck wrote:
> Add support for parsing SEI_TYPE_MASTERING_DISPLAY_INFO and propagate 
> contents into 
> the AVMasteringDisplayMetadata side data. Primaries are ordered in RGB order 
> and
> the values are converted to the natural ranges ([0,1] for CEI 1931 Chroma 
> coords,
> and cd/m^2 for luma).
> 
> Signed-off-by: Neil Birkbeck 
> ---
>  libavcodec/hevc.c | 38 ++
>  libavcodec/hevc.h |  7 +++
>  libavcodec/hevc_sei.c | 22 ++
>  3 files changed, 67 insertions(+)
> 
> diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
> index c245d3b..98cc6f7 100644
> --- a/libavcodec/hevc.c
> +++ b/libavcodec/hevc.c
> @@ -28,6 +28,7 @@
>  #include "libavutil/common.h"
>  #include "libavutil/display.h"
>  #include "libavutil/internal.h"
> +#include "libavutil/mastering_display_metadata.h"
>  #include "libavutil/md5.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
> @@ -2580,6 +2581,43 @@ static int set_side_data(HEVCContext *s)
> s->sei_hflip, s->sei_vflip);
>  }
>  
> +if (s->sei_mastering_display_info_present) {
> +// HEVC uses a g,b,r ordering, which we convert to a more natural 
> r,g,b
> +const int mapping[3] = {2, 0, 1};
> +const float chroma_scale = 5.f;
> +const float luma_scale = 1.f;
> +int i;
> +AVMasteringDisplayMetadata *metadata =
> +av_mastering_display_metadata_create_side_data(out);
> +if (!metadata)
> +return AVERROR(ENOMEM);
> +
> +for (i = 0; i < 3; i++) {
> +const int j = mapping[i];
> +metadata->display_primaries[i][0] =
> +s->display_primaries[j][0] / chroma_scale;
> +metadata->display_primaries[i][1] =
> +s->display_primaries[j][1] / chroma_scale;
> +}

Are display_primaries always rational numbers ?
if so maybe they should be changed to use AVRational

avoiding floats would also make things bit exact across architectures
(without the need for luck)

the same applies to other rational fields

[...]

>  uint8_t* a53_caption;
> diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
> index 07856f2..095c658 100644
> --- a/libavcodec/hevc_sei.c
> +++ b/libavcodec/hevc_sei.c
> @@ -78,6 +78,26 @@ static int decode_nal_sei_decoded_picture_hash(HEVCContext 
> *s)
>  return 0;
>  }
>  
> +static int decode_nal_sei_mastering_display_info(HEVCContext *s)
> +{
> +GetBitContext *gb = >HEVClc->gb;
> +int i;
> +// Mastering primaries
> +for (i = 0; i < 3; i++) {
> +s->display_primaries[i][0] = get_bits(gb, 16);
> +s->display_primaries[i][1] = get_bits(gb, 16);
> +}
> +// White point (x, y)
> +s->white_point[0] = get_bits(gb, 16);
> +s->white_point[1] = get_bits(gb, 16);
> +
> +// Max and min luminance of mastering display

> +s->max_mastering_luminance = get_bits(gb, 32);
> +s->min_mastering_luminance = get_bits(gb, 32);

32 needs get_bits_long()

also the variables are signed int, is that intended

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/hevc Parse SEI_TYPE_MASTERING_DISPLAY_INFO and propagate contents into the AVMasteringDisplayMetadata side data.

2016-01-21 Thread Neil Birkbeck 
Thanks for the quick review, Michael.

The display primaries are encoded as integers (rational with fixed
denominator, I guess) in h265 and HDMI InfoFrames (but in mkv extensions,
they will be floats). Float is sufficient to represent the 16-bit integer
encoding from hevc, so suspect there is not much gain in going more precise
(this info is used as hints when tone mapping from a wider color gamut to a
narrower one).

Regarding the ints, you're correct, we should just use unsigned.

On Thu, Jan 21, 2016 at 3:39 PM, Michael Niedermayer  wrote:

> On Thu, Jan 21, 2016 at 02:26:39PM -0800, Neil Birkbeck wrote:
> > Add support for parsing SEI_TYPE_MASTERING_DISPLAY_INFO and propagate
> contents into
> > the AVMasteringDisplayMetadata side data. Primaries are ordered in RGB
> order and
> > the values are converted to the natural ranges ([0,1] for CEI 1931
> Chroma coords,
> > and cd/m^2 for luma).
> >
> > Signed-off-by: Neil Birkbeck 
> > ---
> >  libavcodec/hevc.c | 38 ++
> >  libavcodec/hevc.h |  7 +++
> >  libavcodec/hevc_sei.c | 22 ++
> >  3 files changed, 67 insertions(+)
> >
> > diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
> > index c245d3b..98cc6f7 100644
> > --- a/libavcodec/hevc.c
> > +++ b/libavcodec/hevc.c
> > @@ -28,6 +28,7 @@
> >  #include "libavutil/common.h"
> >  #include "libavutil/display.h"
> >  #include "libavutil/internal.h"
> > +#include "libavutil/mastering_display_metadata.h"
> >  #include "libavutil/md5.h"
> >  #include "libavutil/opt.h"
> >  #include "libavutil/pixdesc.h"
> > @@ -2580,6 +2581,43 @@ static int set_side_data(HEVCContext *s)
> > s->sei_hflip, s->sei_vflip);
> >  }
> >
> > +if (s->sei_mastering_display_info_present) {
> > +// HEVC uses a g,b,r ordering, which we convert to a more
> natural r,g,b
> > +const int mapping[3] = {2, 0, 1};
> > +const float chroma_scale = 5.f;
> > +const float luma_scale = 1.f;
> > +int i;
> > +AVMasteringDisplayMetadata *metadata =
> > +av_mastering_display_metadata_create_side_data(out);
> > +if (!metadata)
> > +return AVERROR(ENOMEM);
> > +
> > +for (i = 0; i < 3; i++) {
> > +const int j = mapping[i];
> > +metadata->display_primaries[i][0] =
> > +s->display_primaries[j][0] / chroma_scale;
> > +metadata->display_primaries[i][1] =
> > +s->display_primaries[j][1] / chroma_scale;
> > +}
>
> Are display_primaries always rational numbers ?
> if so maybe they should be changed to use AVRational
>
> avoiding floats would also make things bit exact across architectures
> (without the need for luck)
>
> the same applies to other rational fields
>
> [...]
>
> >  uint8_t* a53_caption;
> > diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
> > index 07856f2..095c658 100644
> > --- a/libavcodec/hevc_sei.c
> > +++ b/libavcodec/hevc_sei.c
> > @@ -78,6 +78,26 @@ static int
> decode_nal_sei_decoded_picture_hash(HEVCContext *s)
> >  return 0;
> >  }
> >
> > +static int decode_nal_sei_mastering_display_info(HEVCContext *s)
> > +{
> > +GetBitContext *gb = >HEVClc->gb;
> > +int i;
> > +// Mastering primaries
> > +for (i = 0; i < 3; i++) {
> > +s->display_primaries[i][0] = get_bits(gb, 16);
> > +s->display_primaries[i][1] = get_bits(gb, 16);
> > +}
> > +// White point (x, y)
> > +s->white_point[0] = get_bits(gb, 16);
> > +s->white_point[1] = get_bits(gb, 16);
> > +
> > +// Max and min luminance of mastering display
>
> > +s->max_mastering_luminance = get_bits(gb, 32);
> > +s->min_mastering_luminance = get_bits(gb, 32);
>
> 32 needs get_bits_long()
>
> also the variables are signed int, is that intended
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I have often repented speaking, but never of holding my tongue.
> -- Xenocrates
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel