Re: [FFmpeg-devel] [PATCH] libavutil: add clean aperture (CLAP) side data.

[Neil Birkbeck]

On Mon, Jun 8, 2020 at 7:19 AM Tobias Rapp  wrote:

> On 30.05.2020 12:41, Kieran O Leary wrote:
> > Hi,
> >
> > On Fri 29 May 2020, 22:47 Neil Birkbeck, 
> wrote:
> >
> >> [...]
> >> I've changed the side data to be PixelCrop (instead of CleanAperture)
> given
> >> the intent to reuse as cropping elsewhere.
> >> -For now, I've kept the rational representation--although CLAP seems to
> be
> >> the only required case of it. Maybe Kieran could comment on the
> requirement
> >> of having maintaining the rationals for CLAP (only works on mov to mov
> >> transmuxing).
> >>
> > I'm no expert, but I think a lot of this just comes from video standards
> > that stipulate those rational numbers? I've cc'd tobias Rapp and
> Christoph
> > Gerstbauer of NOA just to bring this to their attention, as I'm pretty
> sure
> > that they use cropping values in AVI, so perhaps all of this could be
> > useful to them in some way.
> >
>
> Hi Kieran,
>
> when digitizing SD video carriers we indeed store some offset
> information in case VBI is preverved (i.e. PAL 720x608). But these
> offset values are currently not stored in the AVI container itself. The
> OpenDML "vprp" chunk defines some offset values but for the purpose of
> compressed image data where the codec implies some multiple-of-N
> height/width dimension on the data. So it did not seem to match our
> use-case.
>
> Besides AVI and the mentioned MKV and MOV formats I remember some
> display offset/cropping information being stored in MXF with the Display
> X/Y-Offset values.
>
> Regardless whether the frame crop/offset values are stored as frame
> fields or side data: how would this information be affected by filters
> like "crop" or "scale"?
>

Thanks for the additional info, Kieran and Tobias.

AVFrame already has cropping fields (AVFrame::crop_{top, bottom, left,
right}), and James Almer's suggestion was that this representation could
replace those. At the moment, when AVCodecContext::apply_cropping is true,
these offsets get applied (and zerod) in libavcodec/decode.c. Currently,
vf_crop adjusts these offsets for HWACCEL pix fmts, and vf_scale_vulkan
also respects (vf_scale doesn't).

Any additional reviewer thoughts on the SideData representation (e.g., crop
fields, currently rational to fully support the mov mux case)?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavutil: add clean aperture (CLAP) side data.

[Neil Birkbeck]

On Mon, May 11, 2020 at 9:37 PM Neil Birkbeck 
wrote:

>
>
> On Wed, May 6, 2020 at 8:45 AM James Almer  wrote:
>
>> On 5/6/2020 12:22 PM, Neil Birkbeck wrote:
>> > On Tue, May 5, 2020 at 5:11 AM Kieran O Leary > >
>> > wrote:
>> >
>> >> Hi,
>> >>
>> >> I broke the threading with my last reply, i apologise. Here goes
>> another
>> >> attempt:
>> >>
>> >> On Tue, Apr 28, 2020 at 6:23 PM Neil Birkbeck > >
>> >> wrote:
>> >>
>> >>> On Tue, Apr 28, 2020 at 3:18 AM Nicolas George 
>> wrote:
>> >>>
>> >>>> Andreas Rheinhardt (12020-04-28):
>> >>>>> That's expected. The patch provided only provides the structure in
>> >>> which
>> >>>>> the values are intended to be exported; it does not add any demuxing
>> >> or
>> >>>>> muxing capabilities for mov or mkv (as you can see from the fact
>> that
>> >>>>> none of these (de)muxers have been changed in the patch).
>> >>>>
>> >>>> Which is something I intended to comment on: adding code without
>> users
>> >>>> is rarely a good idea. I suggest we do not commit until at least one
>> >>>> demuxer use it, preferably at least two. Otherwise, we may realize
>> that
>> >>>> “oh crap, it doesn't work” because of a tiny unforeseen detail.
>> >>>
>> >>>
>> >>> Thanks for the feedback. I also have patches for the demux (MOV/MKV)
>> and
>> >>> mux (MOV/MKV).
>> >>>
>> >>> As there is still the alternative of using the fields in the
>> >>> AVCodecParameters/AVCodecContext, my intention was to keep the first
>> >> patch
>> >>> small to resolve discussion on that point.
>> >>>
>> >>> I've included the patches, if you'd like to try test it, Kieren. I
>> see on
>> >>> your test file that there may be some slight rounding error making
>> output
>> >>> crop 704 not 703 (MKV file ends up with pixel_crop_{left,right} = 8).
>> >>>
>> >>> /ffprobe ../testdata/clap.mov 2>&1 | grep -A1 "Side"
>> >>> Side data:
>> >>>   Clean aperture:[width 41472/59 height:576/1 h_offset:0/1
>> >>> v_offset:0/1]
>> >>> ./ffmpeg -i ../testdata/clap.mov  -vcodec copy -acodec copy
>> /tmp/clap.mkv
>> >>> ./ffprobe /tmp/clap.mkv 2>&1 | grep -A1 "Side"
>> >>> Side data:
>> >>>   Clean aperture:[width 704/1 height:576/1 h_offset:0/1
>> v_offset:0/1]
>> >>>
>> >>
>> >> I have to look deeper into the MKV side of things and most likely
>> raise it
>> >> with the cellar mailing list so that better minds than mine can weigh
>> in. I
>> >> do see that the rounding up to 704 could be an issue alright.
>> >> As for MOV, your patch appears to generate the same output clap values
>> as
>> >> the input, so that's really great! command line and mediainfo trace
>> below:
>> >>
>> >
>> > Thanks for testing, Kieran and for linking the discussion on the cellar
>> > list.
>> >
>> > Any additional thoughts from ffmpeg devs on container-level SideData vs
>> > adding the extra fields into AVCodecParameters/AVCodecContext and
>> plumbing
>> > into the frame instead? I anticipate some situations where there can be
>> > interaction between cropping in bitstream and container-level cropping.
>> > Maybe the best way forward is for me to share some sample patches for
>> the
>> > alternative to validate whether it supports the various use cases
>> > (transmux, decode+crop, any other application level handling), and to
>> > confirm the interface changes for the structs.
>>
>> One option could be to also introduce a frame side data for this new
>> struct and have it replace the AVFrame fields, which would then be
>> deprecated (But of course keep working until removed).
>> This would allow to either inject stream side data from clap atoms and
>> Matroska crop fields into packets (Which would then be propagated to
>> frames to be applied during decoding), or use and export the bitstream
>> cropping information as it's the case right now, all while preventing
>> the addition of new fields to AVCodecParameters.
>>
>>

Re: [FFmpeg-devel] [PATCH] libavutil: add clean aperture (CLAP) side data.

[Neil Birkbeck]

On Wed, May 6, 2020 at 8:45 AM James Almer  wrote:

> On 5/6/2020 12:22 PM, Neil Birkbeck wrote:
> > On Tue, May 5, 2020 at 5:11 AM Kieran O Leary 
> > wrote:
> >
> >> Hi,
> >>
> >> I broke the threading with my last reply, i apologise. Here goes another
> >> attempt:
> >>
> >> On Tue, Apr 28, 2020 at 6:23 PM Neil Birkbeck 
> >> wrote:
> >>
> >>> On Tue, Apr 28, 2020 at 3:18 AM Nicolas George 
> wrote:
> >>>
> >>>> Andreas Rheinhardt (12020-04-28):
> >>>>> That's expected. The patch provided only provides the structure in
> >>> which
> >>>>> the values are intended to be exported; it does not add any demuxing
> >> or
> >>>>> muxing capabilities for mov or mkv (as you can see from the fact that
> >>>>> none of these (de)muxers have been changed in the patch).
> >>>>
> >>>> Which is something I intended to comment on: adding code without users
> >>>> is rarely a good idea. I suggest we do not commit until at least one
> >>>> demuxer use it, preferably at least two. Otherwise, we may realize
> that
> >>>> “oh crap, it doesn't work” because of a tiny unforeseen detail.
> >>>
> >>>
> >>> Thanks for the feedback. I also have patches for the demux (MOV/MKV)
> and
> >>> mux (MOV/MKV).
> >>>
> >>> As there is still the alternative of using the fields in the
> >>> AVCodecParameters/AVCodecContext, my intention was to keep the first
> >> patch
> >>> small to resolve discussion on that point.
> >>>
> >>> I've included the patches, if you'd like to try test it, Kieren. I see
> on
> >>> your test file that there may be some slight rounding error making
> output
> >>> crop 704 not 703 (MKV file ends up with pixel_crop_{left,right} = 8).
> >>>
> >>> /ffprobe ../testdata/clap.mov 2>&1 | grep -A1 "Side"
> >>> Side data:
> >>>   Clean aperture:[width 41472/59 height:576/1 h_offset:0/1
> >>> v_offset:0/1]
> >>> ./ffmpeg -i ../testdata/clap.mov  -vcodec copy -acodec copy
> /tmp/clap.mkv
> >>> ./ffprobe /tmp/clap.mkv 2>&1 | grep -A1 "Side"
> >>> Side data:
> >>>   Clean aperture:[width 704/1 height:576/1 h_offset:0/1
> v_offset:0/1]
> >>>
> >>
> >> I have to look deeper into the MKV side of things and most likely raise
> it
> >> with the cellar mailing list so that better minds than mine can weigh
> in. I
> >> do see that the rounding up to 704 could be an issue alright.
> >> As for MOV, your patch appears to generate the same output clap values
> as
> >> the input, so that's really great! command line and mediainfo trace
> below:
> >>
> >
> > Thanks for testing, Kieran and for linking the discussion on the cellar
> > list.
> >
> > Any additional thoughts from ffmpeg devs on container-level SideData vs
> > adding the extra fields into AVCodecParameters/AVCodecContext and
> plumbing
> > into the frame instead? I anticipate some situations where there can be
> > interaction between cropping in bitstream and container-level cropping.
> > Maybe the best way forward is for me to share some sample patches for the
> > alternative to validate whether it supports the various use cases
> > (transmux, decode+crop, any other application level handling), and to
> > confirm the interface changes for the structs.
>
> One option could be to also introduce a frame side data for this new
> struct and have it replace the AVFrame fields, which would then be
> deprecated (But of course keep working until removed).
> This would allow to either inject stream side data from clap atoms and
> Matroska crop fields into packets (Which would then be propagated to
> frames to be applied during decoding), or use and export the bitstream
> cropping information as it's the case right now, all while preventing
> the addition of new fields to AVCodecParameters.
>
>
I agree that sharing the SideData with the frame could be nice for the
above reasons. I guess any interaction or conflict between bitstream &
container cropping could then get resolved at the decoder (e.g., assuming
that say SPS from h264 and container-level cropping should be composed).


> I would like a developer that makes use of this feature to also comment,
> especially seeing how the AVFrame fields and this clap side data are
> pretty different.
>

The current CleanA

Re: [FFmpeg-devel] [PATCH] libavutil: add clean aperture (CLAP) side data.

[Neil Birkbeck]

On Thu, May 7, 2020 at 10:21 AM Andreas Rheinhardt <
andreas.rheinha...@gmail.com> wrote:

> Neil Birkbeck:
> > On Tue, Apr 28, 2020 at 3:18 AM Nicolas George  wrote:
> >
> >> Andreas Rheinhardt (12020-04-28):
> >>> That's expected. The patch provided only provides the structure in
> which
> >>> the values are intended to be exported; it does not add any demuxing or
> >>> muxing capabilities for mov or mkv (as you can see from the fact that
> >>> none of these (de)muxers have been changed in the patch).
> >>
> >> Which is something I intended to comment on: adding code without users
> >> is rarely a good idea. I suggest we do not commit until at least one
> >> demuxer use it, preferably at least two. Otherwise, we may realize that
> >> “oh crap, it doesn't work” because of a tiny unforeseen detail.
> >
> >
> > Thanks for the feedback. I also have patches for the demux (MOV/MKV) and
> > mux (MOV/MKV).
> >
> > As there is still the alternative of using the fields in the
> > AVCodecParameters/AVCodecContext, my intention was to keep the first
> patch
> > small to resolve discussion on that point.
> >
> > I've included the patches, if you'd like to try test it, Kieren. I see on
> > your test file that there may be some slight rounding error making output
> > crop 704 not 703 (MKV file ends up with pixel_crop_{left,right} = 8).
> >
> > /ffprobe ../testdata/clap.mov 2>&1 | grep -A1 "Side"
> > Side data:
> >   Clean aperture:[width 41472/59 height:576/1 h_offset:0/1
> v_offset:0/1]
> > ./ffmpeg -i ../testdata/clap.mov  -vcodec copy -acodec copy /tmp/clap.mkv
> > ./ffprobe /tmp/clap.mkv 2>&1 | grep -A1 "Side"
> > Side data:
> >   Clean aperture:[width 704/1 height:576/1 h_offset:0/1 v_offset:0/1]
> >
> >
>
> >
> > Write out stream-level AVCleanAperture side data in the muxer.
>
> You should separate the mov and Matroska patches.


I will do this as I incorporate the rest of your suggestions.



>
> > Signed-off-by: Neil Birkbeck 
> > ---
> >  libavformat/matroskaenc.c | 37 +
> >  libavformat/movenc.c  | 28 
> >  2 files changed, 57 insertions(+), 8 deletions(-)
> >
> > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> > index 784973a951..19ff29853e 100644
> > --- a/libavformat/matroskaenc.c
> > +++ b/libavformat/matroskaenc.c
> > @@ -901,6 +901,38 @@ static int mkv_write_video_color(AVIOContext *pb,
> const AVCodecParameters *par,
> >  return 0;
> >  }
> >
> > +static int mkv_write_video_crop(AVIOContext *pb, AVCodecParameters
> *par, AVStream *st)
> > +{
> > +int cropb = 0, cropt = 0, cropl = 0, cropr = 0;
> > +double width, height, h_offset, v_offset;
> > +int side_data_size = 0;
> > +const AVCleanAperture *clap =
> > +(const AVCleanAperture *)av_stream_get_side_data(
> > +st, AV_PKT_DATA_CLEAN_APERTURE, _data_size);
> > +if (!clap)
> > +return 0;
> > +
> > +width = av_q2d(clap->width);
> > +height = av_q2d(clap->height);
> > +h_offset = av_q2d(clap->horizontal_offset);
> > +v_offset = av_q2d(clap->vertical_offset);
> > +cropb = (int)(par->height - (par->height / 2. + v_offset + height /
> 2));
> > +cropt = (int)(par->height / 2. + v_offset - height / 2);
> > +cropr = (int)(par->width - (par->width / 2. + h_offset + width /
> 2));
> > +cropl = (int)(par->width / 2. + h_offset - width / 2);
> > +cropb = FFMAX(cropb, 0);
> > +cropt = FFMAX(cropt, 0);
> > +cropr = FFMAX(cropr, 0);
> > +cropl = FFMAX(cropl, 0);
> > +if (!cropr && !cropl && !cropt && !cropb)
> > +return 0;
> > +put_ebml_uint(pb, MATROSKA_ID_VIDEOPIXELCROPL, cropl);
> > +put_ebml_uint(pb, MATROSKA_ID_VIDEOPIXELCROPR, cropr);
> > +put_ebml_uint(pb, MATROSKA_ID_VIDEOPIXELCROPB, cropb);
> > +put_ebml_uint(pb, MATROSKA_ID_VIDEOPIXELCROPT, cropt);
>
> There is no reason to write all four elements when only some of them are
> different from zero.
>
>
Good point. I will avoid doing this in the revised set of patches.


> > +return 0;
> > +}
> > +
> >  static int mkv_write_video_projection(AVFormatContext *s, AVIOContext
> *pb,
> >const AVStream *st)
> >  {
> > @@ -1287,6 +1319,11 @@ static i

Re: [FFmpeg-devel] [PATCH] libavutil: add clean aperture (CLAP) side data.

[Neil Birkbeck]

On Tue, May 5, 2020 at 5:11 AM Kieran O Leary 
wrote:

> Hi,
>
> I broke the threading with my last reply, i apologise. Here goes another
> attempt:
>
> On Tue, Apr 28, 2020 at 6:23 PM Neil Birkbeck 
> wrote:
>
> > On Tue, Apr 28, 2020 at 3:18 AM Nicolas George  wrote:
> >
> > > Andreas Rheinhardt (12020-04-28):
> > > > That's expected. The patch provided only provides the structure in
> > which
> > > > the values are intended to be exported; it does not add any demuxing
> or
> > > > muxing capabilities for mov or mkv (as you can see from the fact that
> > > > none of these (de)muxers have been changed in the patch).
> > >
> > > Which is something I intended to comment on: adding code without users
> > > is rarely a good idea. I suggest we do not commit until at least one
> > > demuxer use it, preferably at least two. Otherwise, we may realize that
> > > “oh crap, it doesn't work” because of a tiny unforeseen detail.
> >
> >
> > Thanks for the feedback. I also have patches for the demux (MOV/MKV) and
> > mux (MOV/MKV).
> >
> > As there is still the alternative of using the fields in the
> > AVCodecParameters/AVCodecContext, my intention was to keep the first
> patch
> > small to resolve discussion on that point.
> >
> > I've included the patches, if you'd like to try test it, Kieren. I see on
> > your test file that there may be some slight rounding error making output
> > crop 704 not 703 (MKV file ends up with pixel_crop_{left,right} = 8).
> >
> > /ffprobe ../testdata/clap.mov 2>&1 | grep -A1 "Side"
> > Side data:
> >   Clean aperture:[width 41472/59 height:576/1 h_offset:0/1
> > v_offset:0/1]
> > ./ffmpeg -i ../testdata/clap.mov  -vcodec copy -acodec copy /tmp/clap.mkv
> > ./ffprobe /tmp/clap.mkv 2>&1 | grep -A1 "Side"
> > Side data:
> >   Clean aperture:[width 704/1 height:576/1 h_offset:0/1 v_offset:0/1]
> >
>
> I have to look deeper into the MKV side of things and most likely raise it
> with the cellar mailing list so that better minds than mine can weigh in. I
> do see that the rounding up to 704 could be an issue alright.
> As for MOV, your patch appears to generate the same output clap values as
> the input, so that's really great! command line and mediainfo trace below:
>

Thanks for testing, Kieran and for linking the discussion on the cellar
list.

Any additional thoughts from ffmpeg devs on container-level SideData vs
adding the extra fields into AVCodecParameters/AVCodecContext and plumbing
into the frame instead? I anticipate some situations where there can be
interaction between cropping in bitstream and container-level cropping.
Maybe the best way forward is for me to share some sample patches for the
alternative to validate whether it supports the various use cases
(transmux, decode+crop, any other application level handling), and to
confirm the interface changes for the structs.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavutil: add clean aperture (CLAP) side data.

[Neil Birkbeck]

On Tue, Apr 28, 2020 at 3:18 AM Nicolas George  wrote:

> Andreas Rheinhardt (12020-04-28):
> > That's expected. The patch provided only provides the structure in which
> > the values are intended to be exported; it does not add any demuxing or
> > muxing capabilities for mov or mkv (as you can see from the fact that
> > none of these (de)muxers have been changed in the patch).
>
> Which is something I intended to comment on: adding code without users
> is rarely a good idea. I suggest we do not commit until at least one
> demuxer use it, preferably at least two. Otherwise, we may realize that
> “oh crap, it doesn't work” because of a tiny unforeseen detail.


Thanks for the feedback. I also have patches for the demux (MOV/MKV) and
mux (MOV/MKV).

As there is still the alternative of using the fields in the
AVCodecParameters/AVCodecContext, my intention was to keep the first patch
small to resolve discussion on that point.

I've included the patches, if you'd like to try test it, Kieren. I see on
your test file that there may be some slight rounding error making output
crop 704 not 703 (MKV file ends up with pixel_crop_{left,right} = 8).

/ffprobe ../testdata/clap.mov 2>&1 | grep -A1 "Side"
Side data:
  Clean aperture:[width 41472/59 height:576/1 h_offset:0/1 v_offset:0/1]
./ffmpeg -i ../testdata/clap.mov  -vcodec copy -acodec copy /tmp/clap.mkv
./ffprobe /tmp/clap.mkv 2>&1 | grep -A1 "Side"
Side data:
  Clean aperture:[width 704/1 height:576/1 h_offset:0/1 v_offset:0/1]
From d92371cc2de6a671b44d9f6dd9d28544d05b5287 Mon Sep 17 00:00:00 2001
From: Neil Birkbeck 
Date: Sun, 26 Apr 2020 20:42:59 -0700
Subject: [PATCH 3/3] libavformat: Adding MOV/MKV muxer support for CLAP side
 data.

Write out stream-level AVCleanAperture side data in the muxer.

Signed-off-by: Neil Birkbeck 
---
 libavformat/matroskaenc.c | 37 +
 libavformat/movenc.c  | 28 
 2 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 784973a951..19ff29853e 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -901,6 +901,38 @@ static int mkv_write_video_color(AVIOContext *pb, const AVCodecParameters *par,
 return 0;
 }
 
+static int mkv_write_video_crop(AVIOContext *pb, AVCodecParameters *par, AVStream *st)
+{
+int cropb = 0, cropt = 0, cropl = 0, cropr = 0;
+double width, height, h_offset, v_offset;
+int side_data_size = 0;
+const AVCleanAperture *clap =
+(const AVCleanAperture *)av_stream_get_side_data(
+st, AV_PKT_DATA_CLEAN_APERTURE, _data_size);
+if (!clap)
+return 0;
+
+width = av_q2d(clap->width);
+height = av_q2d(clap->height);
+h_offset = av_q2d(clap->horizontal_offset);
+v_offset = av_q2d(clap->vertical_offset);
+cropb = (int)(par->height - (par->height / 2. + v_offset + height / 2));
+cropt = (int)(par->height / 2. + v_offset - height / 2);
+cropr = (int)(par->width - (par->width / 2. + h_offset + width / 2));
+cropl = (int)(par->width / 2. + h_offset - width / 2);
+cropb = FFMAX(cropb, 0);
+cropt = FFMAX(cropt, 0);
+cropr = FFMAX(cropr, 0);
+cropl = FFMAX(cropl, 0);
+if (!cropr && !cropl && !cropt && !cropb)
+return 0;
+put_ebml_uint(pb, MATROSKA_ID_VIDEOPIXELCROPL, cropl);
+put_ebml_uint(pb, MATROSKA_ID_VIDEOPIXELCROPR, cropr);
+put_ebml_uint(pb, MATROSKA_ID_VIDEOPIXELCROPB, cropb);
+put_ebml_uint(pb, MATROSKA_ID_VIDEOPIXELCROPT, cropt);
+return 0;
+}
+
 static int mkv_write_video_projection(AVFormatContext *s, AVIOContext *pb,
   const AVStream *st)
 {
@@ -1287,6 +1319,11 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
 } else if (mkv->mode != MODE_WEBM)
 put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYUNIT, MATROSKA_VIDEO_DISPLAYUNIT_UNKNOWN);
 
+// Write out pixel crop
+ret = mkv_write_video_crop(pb, par, st);
+if (ret < 0)
+return ret;
+
 if (par->codec_id == AV_CODEC_ID_RAWVIDEO) {
 uint32_t color_space = av_le2ne32(par->codec_tag);
 put_ebml_binary(pb, MATROSKA_ID_VIDEOCOLORSPACE, _space, sizeof(color_space));
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 32e8109268..ab2b53cab6 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1847,16 +1847,28 @@ static int mov_write_dvcc_dvvc_tag(AVFormatContext *s, AVIOContext *pb, AVDOVIDe
 
 static int mov_write_clap_tag(AVIOContext *pb, MOVTrack *track)
 {
+const AVCleanAperture* clap =
+(const AVCleanAperture*) av_stream_get_side_data(
+track->st, AV_PKT_DATA_CLEAN_APERTURE, NULL);
+AVRational width = {.num = clap ? clap->width.num : t

Re: [FFmpeg-devel] [PATCH] libavutil: add clean aperture (CLAP) side data.

[Neil Birkbeck]

On Mon, Apr 27, 2020 at 4:38 AM Lynne  wrote:

> Apr 27, 2020, 05:27 by neil.birkb...@gmail.com:
>
> > The clean aperature represents a cropping of the stored image data used
> to
> > relate the image data to a canonical video system and exists as container
> > metadata (see 'clap' section in
> https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html
> )
> >
> > Addition of the side data is a first step towards demuxing CLAP atom
> metadata,
> > helping to resolve https://trac.ffmpeg.org/ticket/7437
> >
> > This CleanAperture representation can also carry PixelCrop fields from
> MKV.
> > Side data was suggested as a way to carry such PixelCrop fields in:
> > https://ffmpeg.org/pipermail/ffmpeg-devel/2016-March/192302.html
> >
> > Transmuxing the side data can then be added (MOV to/from MKV), and
> > auto-application could optionally be enabled like autorotate in
> ffmpeg_filter.c.
> >
>
> We already have frame cropping metadata in the AVFrame structure.
>

Propagating the crop fields from the format/container to AVFrame was my
initial thought. As far as I can tell, currently the cropping fields on
AVFrame are typically set from the coded bitstream. And plumbing from the
container requires the crop fields to be added to AVCodecParameters and to
AVCodecContext, which could then be set on the AVFrame in decode.c.

In part, I based the SideData on some older threads on the topic, but those
threads predate the AVFrame-level cropping. I still see a few a couple of
other potential reasons:
-For the transmux use cases, I was concerned adding the extra fields within
AVCodecParameters/AVCodecContext may confuse the interfaces as to whether
crop/CLAP was destined for the container or the codec. As stream metadata,
it seemed more clear that it should belong in the container
-And a minor one: for the CLAP atom case, on remuxing, the existing integer
crop fields could cause some small sub-pixel loss.

Is it preferred to add the extra fields in AVCodecParameters/AVCodecContext
and plumb them into the frame instead? That should work for the case that
I'm trying to resolve (applying PixelCrop from MKV), but wanted to make
sure the other use cases (e.g., transmux, CLAP atom) worked too.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] libavutil: add clean aperture (CLAP) side data.

[Neil Birkbeck]

The clean aperature represents a cropping of the stored image data used to
relate the image data to a canonical video system and exists as container
metadata (see 'clap' section in 
https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html)

Addition of the side data is a first step towards demuxing CLAP atom metadata,
helping to resolve https://trac.ffmpeg.org/ticket/7437

This CleanAperture representation can also carry PixelCrop fields from MKV.
Side data was suggested as a way to carry such PixelCrop fields in:
https://ffmpeg.org/pipermail/ffmpeg-devel/2016-March/192302.html

Transmuxing the side data can then be added (MOV to/from MKV), and
auto-application could optionally be enabled like autorotate in ffmpeg_filter.c.

Signed-off-by: Neil Birkbeck 
---
 libavcodec/avpacket.c  |  1 +
 libavcodec/packet.h| 12 
 libavformat/dump.c | 15 +
 libavutil/Makefile |  2 ++
 libavutil/clean_aperture.c | 30 ++
 libavutil/clean_aperture.h | 63 ++
 6 files changed, 123 insertions(+)
 create mode 100644 libavutil/clean_aperture.c
 create mode 100644 libavutil/clean_aperture.h

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 55b509108e..2ff779720b 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -397,6 +397,7 @@ const char *av_packet_side_data_name(enum 
AVPacketSideDataType type)
 case AV_PKT_DATA_AFD:return "Active Format 
Description data";
 case AV_PKT_DATA_ICC_PROFILE:return "ICC Profile";
 case AV_PKT_DATA_DOVI_CONF:  return "DOVI configuration 
record";
+case AV_PKT_DATA_CLEAN_APERTURE: return "Clean Aperture";
 }
 return NULL;
 }
diff --git a/libavcodec/packet.h b/libavcodec/packet.h
index 41485f4527..a0f8c29a33 100644
--- a/libavcodec/packet.h
+++ b/libavcodec/packet.h
@@ -282,6 +282,18 @@ enum AVPacketSideDataType {
  */
 AV_PKT_DATA_DOVI_CONF,
 
+/**
+ * This side data contains a crop region (clean aperture) that defines the
+ * region of the stored image that should be cropped.
+ *
+ * It is intended to be used to store crop-related metadata from the
+ * container. For example, the CLAP atom in MOV files, and PixelCrop fields
+ * in MKV.
+ *
+ * The data is of type AVCleanAperture (see libavutil/clean_aperture.h)
+ */
+AV_PKT_DATA_CLEAN_APERTURE,
+
 /**
  * The number of side data types.
  * This is not part of the public API/ABI in the sense that it may
diff --git a/libavformat/dump.c b/libavformat/dump.c
index 5e9a03185f..060e7b2d10 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -26,6 +26,7 @@
 #include "libavutil/display.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/log.h"
+#include "libavutil/clean_aperture.h"
 #include "libavutil/mastering_display_metadata.h"
 #include "libavutil/dovi_meta.h"
 #include "libavutil/mathematics.h"
@@ -402,6 +403,16 @@ static void dump_dovi_conf(void *ctx, AVPacketSideData* sd)
dovi->dv_bl_signal_compatibility_id);
 }
 
+static void dump_clean_aperture(void *ctx, AVPacketSideData *sd)
+{
+AVCleanAperture *clap = (AVCleanAperture *)sd->data;
+av_log(ctx, AV_LOG_INFO, "[width %d/%d height:%d/%d h_offset:%d/%d 
v_offset:%d/%d]",
+   clap->width.num, clap->width.den,
+   clap->height.num, clap->height.den,
+   clap->horizontal_offset.num, clap->horizontal_offset.den,
+   clap->vertical_offset.num, clap->vertical_offset.den);
+}
+
 static void dump_sidedata(void *ctx, AVStream *st, const char *indent)
 {
 int i;
@@ -468,6 +479,10 @@ static void dump_sidedata(void *ctx, AVStream *st, const 
char *indent)
 av_log(ctx, AV_LOG_INFO, "DOVI configuration record: ");
 dump_dovi_conf(ctx, );
 break;
+case AV_PKT_DATA_CLEAN_APERTURE:
+av_log(ctx, AV_LOG_INFO, "Clean aperture:");
+dump_clean_aperture(ctx, );
+break;
 default:
 av_log(ctx, AV_LOG_INFO,
"unknown side data type %d (%d bytes)", sd.type, sd.size);
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 966eec41aa..e9c9b2bff1 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -17,6 +17,7 @@ HEADERS = adler32.h   
  \
   cast5.h   \
   camellia.h\
   channel_layout.h  \
+  clean_aperture.h  \
   common.h  

[FFmpeg-devel] [PATCH] libavcodec/dnxhddata.c: Fix quantization weights for id 1271

[Neil Birkbeck]

From the ST 2019-1:2016 standard,
https://ieeexplore.ieee.org/abstract/document/7513364
in Table D.1, compression ID 1235, 1271 use the same weights.

This fixes what looks like a slight sharpening when decoding
samples encoded as DNxHR HQX 10- or 12-bit from other encoders
(e.g., like from DaVinci Resolve).

Signed-off-by: Neil Birkbeck 
---
 libavcodec/dnxhddata.c | 10 +-
 tests/ref/fate/dnxhr-12bit |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavcodec/dnxhddata.c b/libavcodec/dnxhddata.c
index 154be89860..73260eadba 100644
--- a/libavcodec/dnxhddata.c
+++ b/libavcodec/dnxhddata.c
@@ -25,7 +25,7 @@
 
 /* The quantization tables below are in zigzag order! */
 
-/* Used in CID 1235, 1256, 1270 */
+/* Used in CID 1235, 1256, 1270, 1271 */
 static const uint8_t dnxhd_1235_luma_weight[] = {
  0, 32, 32, 32, 33, 32, 32, 32,
 32, 31, 32, 33, 33, 33, 33, 35,
@@ -37,7 +37,7 @@ static const uint8_t dnxhd_1235_luma_weight[] = {
 50, 50, 53, 55, 55, 56, 60, 60,
 };
 
-/* Used in CID 1235, 1256 */
+/* Used in CID 1235, 1256, 1271 */
 static const uint8_t dnxhd_1235_chroma_weight[] = {
  0, 32, 33, 34, 34, 33, 34, 35,
 37, 40, 43, 42, 39, 38, 39, 41,
@@ -97,7 +97,7 @@ static const uint8_t dnxhd_1238_chroma_weight[] = {
 82, 77, 80, 86, 84, 82, 82, 82,
 };
 
-/* Used in CID 1241, 1271 */
+/* Used in CID 1241 */
 static const uint8_t dnxhd_1241_luma_weight[] = {
  0, 32, 33, 34, 34, 35, 36, 37,
 36, 37, 38, 38, 38, 39, 39, 40,
@@ -109,7 +109,7 @@ static const uint8_t dnxhd_1241_luma_weight[] = {
 48, 46, 47, 48, 48, 49, 49, 49,
 };
 
-/* Used in CID 1241, 1271 */
+/* Used in CID 1241 */
 static const uint8_t dnxhd_1241_chroma_weight[] = {
  0, 32, 36, 38, 37, 37, 40, 41,
 40, 40, 42, 42, 41, 41, 41, 41,
@@ -1047,7 +1047,7 @@ const CIDEntry ff_dnxhd_cid_table[] = {
   { 0 }, { 57344, 255} },
 { 1271, DNXHD_VARIABLE, DNXHD_VARIABLE, DNXHD_VARIABLE, DNXHD_VARIABLE,
   0, 6, DNXHD_VARIABLE, 4,
-  dnxhd_1241_luma_weight, dnxhd_1241_chroma_weight,
+  dnxhd_1235_luma_weight, dnxhd_1235_chroma_weight,
   dnxhd_1235_dc_codes, dnxhd_1235_dc_bits,
   dnxhd_1235_ac_codes, dnxhd_1235_ac_bits, dnxhd_1235_ac_info,
   dnxhd_1235_run_codes, dnxhd_1235_run_bits, dnxhd_1235_run,
diff --git a/tests/ref/fate/dnxhr-12bit b/tests/ref/fate/dnxhr-12bit
index eb5716780c..c6e8fd99ae 100644
--- a/tests/ref/fate/dnxhr-12bit
+++ b/tests/ref/fate/dnxhr-12bit
@@ -3,4 +3,4 @@
 #codec_id 0: rawvideo
 #dimensions 0: 1920x1080
 #sar 0: 1/1
-0,  0,  0,1,  8294400, 0x31bfa8f1
+0,  0,  0,1,  8294400, 0xbe1dc546
-- 
2.26.1.301.g55bc3eb7cb9-goog

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: Demux the PixelCrop* values

[Neil Birkbeck]

On Fri, Apr 15, 2016 at 4:06 AM wm4  wrote:

> On Wed, 6 Apr 2016 17:09:21 -0600
> Nic Wolfe  wrote:
>
> > Thanks for elaborating wm4. Out of curiosity when you say "generic"
> > and "normal" metadata what do you mean? Are you talking about using
> > generic key names or is there another way to store metadata?
> >
> > I have attached a patch which adds a new AVPacketSideDataType and uses
> > that type to store crop values in the AVStream's side data if they're
> > found during Matroska demuxing. I have included a comment in avcodec.h
> > with some of the info Dave shared above but I'm not sure if that is
> > sufficient to call this "well defined".
> >
> > I'm aware that this is a complex topic and this patch is certainly not
> > comprehensive but hopefully it is a step in the right direction.
>
> I think this is much better. The problem are still the semantics. I
> tried to discuss it in Libav, but nothing conclusive has emerged.
>
> Maybe you could send this patch to libav-devel for discussion? (I feel
> a bit sorry for playing this ping pong with a first time contributor.
> If anyone has a better idea how to continue this, please make a
> suggestion.)
>
> By the way, it's not a good idea to send base64 encoded patches. I
> don't think most mail clients can expand them automatically. It
> makes review harder. Just attach them normally. Also, try to avoid
> top-posting.
>

Reviving this thread, as it seems to have the most context on the topic of
PixelCrop in MKV.

I was also investigating adding support for PixelCrop* fields (
https://matroska.org/technical/specs/index.html) for a particular case of
cropping additional padding off the bottom of the image (e.g.,
PixelWidth=1920, PixelHeight=1088, PixelCropBottom=8,
PixelCrop{Top,Right,Left}=0, DisplayWidth=1920, DisplayHeight=1080) which
is required due to a hardware constraint (and codec bitrstream doesn't
support the crop, unlike say h264/hevc). For my use-case, propagating the
MKV fields through to the AVFrame::crop* and silently applying the crop
would be ideal behavior, but I realize that may not generalize to others.

It seems there is still a bunch of complexity around the various edge cases
and interaction with metadata in the container/codec (e.g., which one takes
precedent?).  And the PixelCrop fields also come up in (
https://trac.ffmpeg.org/ticket/7437), where it seems the fields might be a
natural destination for contents of mov/mp4 "clap" atom (
https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html#//apple_ref/doc/uid/TP4939-CH205-125850).
This seems partially relevant, as perhaps a common SideData element can
contain the "clap" and  "PixelCrop*" (if possible)--making the interface
more consistent for applications and transmuxing cases

Is an incremental way forward to:
-Support application level support by extract the PixelCrop fields and
exporting as AVStream side data (perhaps works for 'clap' atom too). [say
by revisiting the AVPacketSideData patch in this thread]
-Support transmuxing use case: add support to muxers, mapping between
relevant containers, where/if applicable.
-Support auto-apply decode case. I guess this is close to what autorotate
does. Need to be sure to strip strip metadata so it doesn't get muxed into
output.


Any alternatives, or objections, to at least exporting these fields as
stream side data as a first step?


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/proresdec2: only set avctx->color* when values are specified

[Neil Birkbeck]

On Fri, Jan 18, 2019 at 7:48 AM Vittorio Giovara 
wrote:

> On Fri, Jan 18, 2019 at 6:43 AM Carl Eugen Hoyos 
> wrote:
>
> > 2019-01-18 4:48 GMT+01:00, Neil Birkbeck :
> > > On Thu, Jan 17, 2019 at 7:43 PM Neil Birkbeck  >
> > > wrote:
> > >
> > >> This allows preservation of color values set from the container,
> > >> while still letting the bitstream take precedent when its values
> > >> are specified to some actual value (e.g., not *UNSPECIFIED).
> > >>
> > >> Signed-off-by: Neil Birkbeck 
> > >> ---
> > >>  libavcodec/proresdec2.c | 9 ++---
> > >>  1 file changed, 6 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
> > >> index 6209c229c9..662ca7d48b 100644
> > >> --- a/libavcodec/proresdec2.c
> > >> +++ b/libavcodec/proresdec2.c
> > >> @@ -262,9 +262,12 @@ static int decode_frame_header(ProresContext
> *ctx,
> > >> const uint8_t *buf,
> > >>  }
> > >>  }
> > >>
> > >> -avctx->color_primaries = buf[14];
> > >> -avctx->color_trc   = buf[15];
> > >> -avctx->colorspace  = buf[16];
> > >> +if (buf[14] != AVCOL_PRI_UNSPECIFIED)
> > >> +avctx->color_primaries = buf[14];
> > >> +if (buf[15] != AVCOL_TRC_UNSPECIFIED)
> > >> +avctx->color_trc   = buf[15];
> > >> +if (buf[16] != AVCOL_SPC_UNSPECIFIED)
> > >> +avctx->colorspace  = buf[16];
> > >>  avctx->color_range = AVCOL_RANGE_MPEG;
> > >>
> > >>  ptr   = buf + 20;
> > >> --
> > >> 2.20.1.321.g9e740568ce-goog
> > >>
> > >>
> > > To add a bit more context. The patch is a fix for the case when prores
> > > bitstream code points are explicitly set to unspecified and are
> > overriding
> > > what may have been in the container (unlike h264/h265 where such values
> > can
> > > behind the color_description flag, these fields always must be present
> in
> > > the prores header).
> >
> > Isn't this even more of a reason to prefer the container value over
> > the bitstream value?
> >
>
> Absolutely not
>
I was initially going to check and see if preferring the container (when
set) was a valid alternative, since that was what the old behavior was. But
I couldn't find precedent for that elsewhere in the code (except when the
codec was just assuming some fixed constant not specified in the
bitstream). If a library user wants to prefer/detect container metadata, I
take it they have to avformat_open_input, inspect metadata before
find_stream_info, and use their application-specific logic to do add any
bitstream filters or adjust parameters of the filter graph?

For posterity, there are a couple more slightly relevant links I found when
looking for an authoritative reference on such inconsistencies in prores. I
only found [1] that confirms the tooling/workflows were inconsistent at
some point, and [2] which suggested that the colr atom took precedence over
headers on whatever viewer they were talking about (presumably quicktime,
on mac).

[1] https://github.com/bbc/qtff-parameter-editor
[2]
https://www.drastic.tv/support-59/supporttipstechnical/202-prores-colour-shifts-in-post-production
From ab77fa3035bace9dfff3b9f83e623e823cac80cf Mon Sep 17 00:00:00 2001
From: Neil Birkbeck 
Date: Tue, 15 Jan 2019 17:19:44 -0800
Subject: [PATCH] avcodec/proresdec2: only set avctx->color* when values are
 specified

This allows preservation of color values set from the container,
while still letting the bitstream take precedent when its values
are specified to some actual value (e.g., not *UNSPECIFIED).

The patch is a fix for the case when prores bitstream code points
are explicitly set to unspecified and are overriding what may have
been in the container (unlike h264/h265 where such values can behind
the color_description flag, these fields always must be present in
the prores header). Of course, ideally these should be the same.

We noticed this inconsistency on some HDR content, as prior to
https://github.com/FFmpeg/FFmpeg/commit/81d78fe13bc1fd94845c002f3439fe44d9e9e0d9
the color information in the prores bitstream (which may have been
unspecified) was simply ignored.

In practice, I guess some workflows may not have known about the new
code points and put unspecified in the bitstream (or worse where some
headers will contain non-HDR code points). A bit more info on that topic
related to HDR workflows here:  https://github.com/bbc/qtff-parameter-editor

The patch seem

Re: [FFmpeg-devel] [PATCH] avcodec/proresdec2: only set avctx->color* when values are specified

[Neil Birkbeck]

On Thu, Jan 17, 2019 at 7:43 PM Neil Birkbeck 
wrote:

> This allows preservation of color values set from the container,
> while still letting the bitstream take precedent when its values
> are specified to some actual value (e.g., not *UNSPECIFIED).
>
> Signed-off-by: Neil Birkbeck 
> ---
>  libavcodec/proresdec2.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
> index 6209c229c9..662ca7d48b 100644
> --- a/libavcodec/proresdec2.c
> +++ b/libavcodec/proresdec2.c
> @@ -262,9 +262,12 @@ static int decode_frame_header(ProresContext *ctx,
> const uint8_t *buf,
>  }
>  }
>
> -avctx->color_primaries = buf[14];
> -avctx->color_trc   = buf[15];
> -avctx->colorspace  = buf[16];
> +if (buf[14] != AVCOL_PRI_UNSPECIFIED)
> +avctx->color_primaries = buf[14];
> +if (buf[15] != AVCOL_TRC_UNSPECIFIED)
> +avctx->color_trc   = buf[15];
> +if (buf[16] != AVCOL_SPC_UNSPECIFIED)
> +avctx->colorspace  = buf[16];
>  avctx->color_range = AVCOL_RANGE_MPEG;
>
>  ptr   = buf + 20;
> --
> 2.20.1.321.g9e740568ce-goog
>
>
To add a bit more context. The patch is a fix for the case when prores
bitstream code points are explicitly set to unspecified and are overriding
what may have been in the container (unlike h264/h265 where such values can
behind the color_description flag, these fields always must be present in
the prores header). Of course, ideally these should be the same.

We noticed this inconsistency on some HDR content, as prior to
https://github.com/FFmpeg/FFmpeg/commit/81d78fe13bc1fd94845c002f3439fe44d9e9e0d9
the color information in the prores bitstream (which may have been
unspecified) was simply ignored.

In practice, I guess some workflows may not have known about the new code
points and put unspecified in the bitstream (or worse where some headers
will contain non-HDR code points).

The patch seemed like a situation where we could resolve the inconsistency
in favor of the container (given my understanding, and from what I see in
the code, I'm assuming the codec is typically given preference). But I'm
interested in hearing ffmpeg-devel's opinions on whether this is most
consistent (actually, for the HDR files that I've seen, the container is
correct--although I'm sure there are cases where the opposite is true).

I guess the most closely related discussion about codecs overriding these
types of values is here
https://patchwork.ffmpeg.org/patch/11279/
but this case seemed a bit different.

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

[FFmpeg-devel] [PATCH] avcodec/proresdec2: only set avctx->color* when values are specified

[Neil Birkbeck]

This allows preservation of color values set from the container,
while still letting the bitstream take precedent when its values
are specified to some actual value (e.g., not *UNSPECIFIED).

Signed-off-by: Neil Birkbeck 
---
 libavcodec/proresdec2.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
index 6209c229c9..662ca7d48b 100644
--- a/libavcodec/proresdec2.c
+++ b/libavcodec/proresdec2.c
@@ -262,9 +262,12 @@ static int decode_frame_header(ProresContext *ctx, const 
uint8_t *buf,
 }
 }
 
-avctx->color_primaries = buf[14];
-avctx->color_trc   = buf[15];
-avctx->colorspace  = buf[16];
+if (buf[14] != AVCOL_PRI_UNSPECIFIED)
+avctx->color_primaries = buf[14];
+if (buf[15] != AVCOL_TRC_UNSPECIFIED)
+avctx->color_trc   = buf[15];
+if (buf[16] != AVCOL_SPC_UNSPECIFIED)
+avctx->colorspace  = buf[16];
 avctx->color_range = AVCOL_RANGE_MPEG;
 
 ptr   = buf + 20;
-- 
2.20.1.321.g9e740568ce-goog

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

Re: [FFmpeg-devel] [PATCH] swscale/utils: Remove bpc==8 gating init_range_convert

[Neil Birkbeck]

On Sat, Dec 2, 2017 at 5:25 PM, Carl Eugen Hoyos <ceffm...@gmail.com> wrote:

> 2017-12-01 20:08 GMT+01:00 Neil Birkbeck <neil.birkb...@gmail.com>:
> > On Thu, Nov 30, 2017 at 9:52 AM, Michael Niedermayer wrote:
>
> >> > For that sample, I feel like it may be incorrectly tagged as pc/full.
> >>
> >> is it stored in the file or taken from:
> >> ff_generate_avci_extradata()
> >> maybe theres a bug in the AVCIntra handling
> >
> > It seems avci100_1080i_extradata may be the one that is signalling full
> > range for the AVCI100.mov sample. I tested changing the range flag:
> > -0x3c, 0x60, 0x20, 0x20, 0x28, 0x00, 0x00, 0x03,
> > +0x3c, 0x20, 0x20, 0x20, 0x28, 0x00, 0x00, 0x03,
>
> If you believe it is safe (or a bug), please change it.
>

I've looked at a few avci-100 files that I could find, and the ones I came
across had data that appeared to be tv range. Unless there is some spec
that says it should be pc/full, I'd say tv (or unspecified) may be a better
default. Let me look into the other avci modes to confirm.

I also attached an updated patch with fate tests. In it, for each
configuration {8-bit/10-bit, scaled/unscaled}, the checksum should be the
same when in_range is set explicitly (or implicitly). And output explicit
to tv or unset is is the same. There is still the issue of whether having
out_range be "unspecified" should give "tv" range (that is what happens for
8-bit at the moment).

Samples are here:
https://github.com/nbirkbeck/ffmpeg-test-samples/tree/master/color-range/data

In the patch, I'd put them in ffmpeg-synthetic/color.
From caa75c6d5d323415647c93d1c7c1d5326f8cf825 Mon Sep 17 00:00:00 2001
From: Neil Birkbeck <neil.birkb...@gmail.com>
Date: Tue, 28 Nov 2017 15:56:12 -0800
Subject: [PATCH] swscale/utils: Remove bpc==8 gating init_range_convert

On higher bit depth YUV inputs with range metadata signaled as PC/full levels,
this change makes the results of scaling without explicitly
setting the input range on vf_scale as if it were explicitly set.

For example, the results with implicit color settings from the frame:
-vf scale=-1:-1:out_range=mpeg,format=yuv420p

Are the same as the correct result when set explicitly in the scaler:
-vf scale=-1:-1:in_range=jpeg:out_range=mpeg,format=yuv420p

The results are consistent with a similar yuv420p(pc) test input
(e.g., implicit and explicit setting of in_range on vf_scale both work).

Fate passes without the checks (or with a more specific check for >= 8).
If this seems sane, I'll write some tests.

I tried to reproduce the old results from before and after the commit
that I think the previous comment was referring to
4959a4fcf76e7c595dbb23c4e3bf59abf2e60ea4
but failed to repro (I may be testing the wrong thing). Using the samples
from (https://trac.ffmpeg.org/ticket/2939), without the check:
ffmpeg -i /tmp/progressive.jpg -vf format=rgb24 /tmp/progressive.png
is still treated as full range input (treating it as studio causes clipping
in the rgb).

There are still some other edge cases where range conversion doesn't work
unless explicitly set (e.g., when no scale is happening)

New fate samples from:
https://github.com/nbirkbeck/ffmpeg-test-samples/tree/master/color-range/data

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libswscale/utils.c |  4 +-
 tests/fate/filter-video.mak| 67 ++
 .../filter-scale-yuv444p-pc-scaled-in-pc-out-tv| 10 
 ...filter-scale-yuv444p-pc-scaled-in-pc-out-unspec | 10 
 ...filter-scale-yuv444p-pc-scaled-in-unspec-out-tv | 10 
 ...er-scale-yuv444p-pc-scaled-in-unspec-out-unspec | 10 
 .../filter-scale-yuv444p-pc-unscaled-in-pc-out-tv  | 10 
 ...lter-scale-yuv444p-pc-unscaled-in-pc-out-unspec | 10 
 ...lter-scale-yuv444p-pc-unscaled-in-unspec-out-tv | 10 
 ...-scale-yuv444p-pc-unscaled-in-unspec-out-unspec | 10 
 .../filter-scale-yuv444p10-pc-scaled-in-pc-out-tv  | 10 
 ...lter-scale-yuv444p10-pc-scaled-in-pc-out-unspec | 10 
 ...lter-scale-yuv444p10-pc-scaled-in-unspec-out-tv | 10 
 ...-scale-yuv444p10-pc-scaled-in-unspec-out-unspec | 10 
 ...filter-scale-yuv444p10-pc-unscaled-in-pc-out-tv | 10 
 ...er-scale-yuv444p10-pc-unscaled-in-pc-out-unspec | 10 
 ...er-scale-yuv444p10-pc-unscaled-in-unspec-out-tv | 10 
 ...cale-yuv444p10-pc-unscaled-in-unspec-out-unspec | 10 
 18 files changed, 228 insertions(+), 3 deletions(-)
 create mode 100644 tests/ref/fate/filter-scale-yuv444p-pc-scaled-in-pc-out-tv
 create mode 100644 tests/ref/fate/filter-scale-yuv444p-pc-scaled-in-pc-out-unspec
 create mode 100644 tests/ref/fate/filter-scale-yuv444p-pc-scaled-in-unspec-out-tv
 create mode 100644 tests/ref/fate/filter-scale-yuv444p-pc-scaled-in-unspec-out-unspec
 create mode 100644 tests/ref/fate/filter-scale-yuv444p-pc-un

Re: [FFmpeg-devel] [PATCH] swscale/utils: Remove bpc==8 gating init_range_convert

[Neil Birkbeck]

On Thu, Nov 30, 2017 at 9:52 AM, Michael Niedermayer  wrote:

> > > Perfect, thanks Michael. Let me check those samples out.
>
> there are 2 more in 2939 which change:
> https://trac.ffmpeg.org/ticket/2939
>

It seems the swscale_unscaled code paths do not get reconfigured when
sws_setColorSpaceDetails is called. The unscaled code path also seems to
get called for yuv420p16le->yuv420p, so the patch only affected those
samples when doing some scaling. But the newer results looks consistent
with direct conversion to rgb (and is consistent when I decode the samples
in matlab).


> > >
> > For that sample, I feel like it may be incorrectly tagged as pc/full.
>
> is it stored in the file or taken from:
> ff_generate_avci_extradata()
> maybe theres a bug in the AVCIntra handling
>
> It seems avci100_1080i_extradata may be the one that is signalling full
range for the AVCI100.mov sample. I tested changing the range flag:
-0x3c, 0x60, 0x20, 0x20, 0x28, 0x00, 0x00, 0x03,
+0x3c, 0x20, 0x20, 0x20, 0x28, 0x00, 0x00, 0x03,
There is an unused ACLR atom in the mov that also appears to signal full
range (parsing of that atom is skipped for h264)

>
> > > Report generated with:
> > > https://raw.githubusercontent.com/nbirkbeck/ffmpeg-test-
> > > samples/master/color-range/run.sh
>
> Can you turn this into a fate test ?
>

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

Re: [FFmpeg-devel] [PATCH] swscale/utils: Remove bpc==8 gating init_range_convert

[Neil Birkbeck]

On Wed, Nov 29, 2017 at 7:40 PM, Neil Birkbeck <neil.birkb...@gmail.com>
wrote:

>
>>
>> If you are searching for a case where the patch makes a difference
>> one is:
>> ./ffmpeg -i ~/tickets/4493/AVCI100.mov out.nut
>> file should be here:
>> http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket524/
>>
>> if you want more cases that change, ill see if i can find more
>>
>
> Perfect, thanks Michael. Let me check those samples out.
>
>
For that sample, I feel like it may be incorrectly tagged as pc/full.
Looking at the histogram of the original there is no data in low range and
a peak due to clipping near where you'd expect higher up for studio/mpeg
range
ffplay /tmp/AVCI100.mov -vf histogram

And scaling, treating the input as studio/mpeg and outputting full range,
stretches y to cover the entire range:
ffplay /tmp/AVCI100.mov -vf
scale=-1:-1:in_range=mpeg:out_range=jpeg,format=yuv422p10le,histogram

This is a real concern though. I don't have a good feel for how many higher
bit depth files are incorrectly labelled as pc/full.

Here is a comparison of what I was described in the commit log.
>
> The naming of the files images are 
> ${pixfmt}_${scaled}_${in_range}_${out_range}
> for with/without the patch:
> https://rawgit.com/nbirkbeck/ffmpeg-test-samples/master/
> color-range/results/report.html
>
> My concern was the yuv444p10_${scaled}_jpeg_mpeg (explicit settings), give
> different results than the implicit yuv444p10_unscaled_unspec_mpeg ones for
> high bit depth. And the high bit depth is results are in general different
> than the low bit depth ones.
>
> Report generated with:
> https://raw.githubusercontent.com/nbirkbeck/ffmpeg-test-
> samples/master/color-range/run.sh
> Using these test files:
> https://github.com/nbirkbeck/ffmpeg-test-samples/tree/
> master/color-range/data
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] swscale/utils: Remove bpc==8 gating init_range_convert

[Neil Birkbeck]

>
>
>
> If you are searching for a case where the patch makes a difference
> one is:
> ./ffmpeg -i ~/tickets/4493/AVCI100.mov out.nut
> file should be here:
> http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket524/
>
> if you want more cases that change, ill see if i can find more
>

Perfect, thanks Michael. Let me check those samples out.

Here is a comparison of what I was described in the commit log.

The naming of the files images are
${pixfmt}_${scaled}_${in_range}_${out_range} for with/without the patch:
https://rawgit.com/nbirkbeck/ffmpeg-test-samples/master/color-range/results/report.html


My concern was the yuv444p10_${scaled}_jpeg_mpeg (explicit settings), give
different results than the implicit yuv444p10_unscaled_unspec_mpeg ones for
high bit depth. And the high bit depth is results are in general different
than the low bit depth ones.

Report generated with:
https://raw.githubusercontent.com/nbirkbeck/ffmpeg-test-samples/master/color-range/run.sh
Using these test files:
https://github.com/nbirkbeck/ffmpeg-test-samples/tree/master/color-range/data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] swscale/utils: Remove bpc==8 gating init_range_convert

[Neil Birkbeck]

On Tue, Nov 28, 2017 at 5:14 PM, Neil Birkbeck <neil.birkb...@gmail.com>
wrote:

> -//The srcBpc check is possibly wrong but we seem to lack a definitive
> reference to test this
> -//and what we have in ticket 2939 looks better with this check
> -if (need_reinit && (c->srcBpc == 8 || !isYUV(c->srcFormat)))
> +if (need_reinit)
>  ff_sws_init_range_convert(c);
>

Wanted to check and see if someone more familiar with this condition can
comment as to whether some parts of it may still be necessary.

Also, this may also need to be coupled with a change to vf_scale.
E.g., at the moment, I think scaling >10bit YUV without explicit _range
settings may not change color range of the data (but it may assign
incorrect color_range flag to the frame).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] swscale/utils: Remove bpc==8 gating init_range_convert

[Neil Birkbeck]

On higher bit depth YUV inputs with range metadata signaled as PC/full levels,
this change makes the results of scaling without explicitly
setting the input range on vf_scale as if it were explicitly set.

For example, the results with implicit color settings from the frame:
-vf scale=-1:-1:out_range=mpeg,format=yuv420p

Are the same as the correct result when set explicitly in the scaler:
-vf scale=-1:-1:in_range=jpeg:out_range=mpeg,format=yuv420p

The results are consistent with a similar yuv420p(pc) test input
(e.g., implicit and explicit setting of in_range on vf_scale both work).

Fate passes without the checks (or with a more specific check for >= 8).
If this seems sane, I'll write some tests.

I tried to reproduce the old results from before and after the commit
that I think the previous comment was referring to
4959a4fcf76e7c595dbb23c4e3bf59abf2e60ea4
but failed to repro (I may be testing the wrong thing). Using the samples
from (https://trac.ffmpeg.org/ticket/2939), without the check:
ffmpeg -i /tmp/progressive.jpg -vf format=rgb24 /tmp/progressive.png
is still treated as full range input (treating it as studio causes clipping
in the rgb).

There are still some other edge cases where range conversion doesn't work
unless explicitly set (e.g., when no scale is happening)

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libswscale/utils.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/libswscale/utils.c b/libswscale/utils.c
index 4df09306d3..f900e3bdef 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -883,9 +883,7 @@ int sws_setColorspaceDetails(struct SwsContext *c, const 
int inv_table[4],
 c->srcRange   = srcRange;
 c->dstRange   = dstRange;
 
-//The srcBpc check is possibly wrong but we seem to lack a definitive 
reference to test this
-//and what we have in ticket 2939 looks better with this check
-if (need_reinit && (c->srcBpc == 8 || !isYUV(c->srcFormat)))
+if (need_reinit)
 ff_sws_init_range_convert(c);
 
 c->dstFormatBpp = av_get_bits_per_pixel(desc_dst);
-- 
2.15.0.417.g466bffb3ac-goog

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

Re: [FFmpeg-devel] [PATCH] avformat/utils: check for overflow before reallocating side data

[Neil Birkbeck]

On Sat, Nov 19, 2016 at 3:28 PM, James Almer  wrote:

> On 11/19/2016 7:19 PM, Michael Niedermayer wrote:
> > On Sat, Nov 19, 2016 at 03:09:15PM -0300, James Almer wrote:
> >> This makes av_stream_add_side_data() consistent with
> av_packet_add_side_data().
> >>
> >> Signed-off-by: James Almer 
> >> ---
> >>  libavformat/utils.c | 5 -
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > LGTM
> >
> > thx
>
> Pushed, Thanks.


Isn't the realloc missing brackets around the num elements:
   tmp = av_realloc(st->side_data, (st->nb_side_data + 1) * sizeof(*tmp));
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc.c: use metadata key/value to set mastering metadata

[Neil Birkbeck]

On Wed, Jun 8, 2016 at 11:27 PM, Dave Rice <d...@dericed.com> wrote:

>
> > On Jun 8, 2016, at 11:03 PM, Neil Birkbeck <neil.birkb...@gmail.com>
> wrote:
> >
> > The most recent patch should still apply cleanly (unless we have a
> > better way to set these elements). Thanks
>
> There are specific mastering metadata elements as recently defined in the
> Matroska specification [1]. It would be better to use these defined
> elements from the specification rather than somehow fit this data into tags.
> Dave Rice
>
> [1] https://matroska.org/technical/specs/index.html#MasteringMetadata


Yes, the new elements are read and written, and are propagated within
ffmpeg in side data. This patch is to address how we can manually specify
those values when creating a new file from an input that doesn't have the
metadata already. The current patch doesn't write them to tags in the
output file, but uses the "metadata" command line. I kept this like
stereo_mode, but maybe we should instead use specific muxer option to do
this? Or perhaps there is another generic way to instantiate side data from
command line upstream from the muxer that I am missing.


>
>
> > On Tue, Apr 19, 2016 at 8:12 AM, Neil Birkbeck <neil.birkb...@gmail.com>
> wrote:
> >> Updated patch attached.
> >>
> >> On Sat, Apr 16, 2016 at 7:08 PM, Michael Niedermayer
> >> <mich...@niedermayer.cc> wrote:
> >>> On Sun, Apr 03, 2016 at 03:38:33PM -0700, Neil Birkbeck wrote:
> >>>> Use "master_display" key/value pair to specify mastering metadata in a
> >>>> similar formatting as accepted by libx265 (unless there is some other
> >>>> generic way to add side data to a stream from command line).
> >>>> Currently, the packet side data propagates from an input file to
> >>>> output file if it is transmuxed (mkv -> mkv). Perhaps we want to also
> >>>> use this same metadata key in matroskadec to also allow for the
> >>>> metadata to propagate during transcoding.
> >>>
> >>>> matroskaenc.c |   51 ++
> +++++
> >>>> 1 file changed, 47 insertions(+), 4 deletions(-)
> >>>> d92564f3ec6cf08430a79b64f4d1ec304637afe1
> 0001-lavf-matroskaenc.c-use-metadata-key-value-to-set-mas.patch
> >>>> From b30d80f6ba4b09811039f64af3e7f709d86df5fe Mon Sep 17 00:00:00
> 2001
> >>>> From: Neil Birkbeck <neil.birkb...@gmail.com>
> >>>> Date: Fri, 1 Apr 2016 17:02:42 -0700
> >>>> Subject: [PATCH] lavf/matroskaenc.c: use metadata key/value to set
> mastering
> >>>> metadata
> >>>>
> >>>> Add key/value metadata interface to allow command line setting of
> AVMasteringDisplayMetadata. The formatting is the same as the option in
> libx265.
> >>>>
> >>>> Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
> >>>
> >>> this does not apply cleanly anymore after codecpar
> >>>
> >>> Applying: lavf/matroskaenc.c: use metadata key/value to set mastering
> metadata
> >>> Using index info to reconstruct a base tree...
> >>> Falling back to patching base and 3-way merge...
> >>> Auto-merging libavformat/matroskaenc.c
> >>> CONFLICT (content): Merge conflict in libavformat/matroskaenc.c
> >>> Failed to merge in the changes.
> >>> Patch failed at 0001 lavf/matroskaenc.c: use metadata key/value to set
> mastering metadata
> >>> When you have resolved this problem run "git am --resolved".
> >>> If you would prefer to skip this patch, instead run "git am --skip".
> >>> To restore the original branch and stop patching run "git am --abort".
> >>>
> >>>
> >>> [...]
> >>>
> >>> --
> >>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC7
> 87040B0FAB
> >>>
> >>> Democracy is the form of government in which you can choose your
> dictator
> >>>
> >>> ___
> >>> 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
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> htt

Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc.c: add early support for colour elements

[Neil Birkbeck]

>
>
> This seems to have been made official now, as usual under version 4
> like every other new element.
> https://github.com/Matroska-Org/libmatroska/commit/
> cd05bc14b42bd88218c61837208fa2b6c79b1de4
>
> Could you confirm there were no changes to the draft and remove the
> experimental checks for both decoder and encoder?
>

There were no more changes to the draft that affected what was in this
patch. However, from the muxer side, the last I checked there were still
players that bailed out on any unknown elements (e.g., chrome, when playing
back through media source extensions). And on the decoder, I've noticed an
issue when the matroska_track_video_color is not specified: the defaults
specified in the EbmlSyntax for this element were not propagated to the
nested struct. Let me address that one first.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc.c: use dyn_buf to write header elements for non-seekable outputs.

[Neil Birkbeck]

On Sun, Oct 9, 2016 at 6:45 PM, James Almer <jamr...@gmail.com> wrote:

> On 10/9/2016 10:13 PM, Neil Birkbeck wrote:
> > For non-seekable output files, larger elements written in write_header
> > (like larger attachments, or possibly many tags) can go over
> > IO_BUFFER_SIZE meaning seeking back to write valid seek head may fail.
>
> Ever since the CRC32 implementation, matroska files written to non
> seekable output already do pretty much everything with dynamic buffers.
> This behavior you mention should have been fixed by it.
>
>
> > Fate test checksums are the same when "-write_crc32=0". Adding dyn_buf
> > causes buffer to be seekable and so crc32 is written now (causing the
> > diffs).
> >
> > Example to reproduce:
> >
> >   mkfifo /tmp/a.fifo
> >   dd if=/dev/zero bs=1024 count=40 > /tmp/data
> >   ffmpeg -nostats -nostdin -y -f lavfi -i testsrc -vframes 1 \
> >  -attach /tmp/data -metadata:s:1 mimetype=test/data \
> >  -f matroska /tmp/a.fifo & cat /tmp/a.fifo > /tmp/a.mkv
> >   ffprobe /tmp/a.mkv
> >
> > Previously would generate file like:
> >  Duration: N/A, start: 0.00, bitrate: N/A
> > Stream #0:0: Video: mpeg4 (Simple Profile), yuv420p, 320x240 [SAR
> 1:1 DAR 4:3], 25 fps, 25 tbr, 1k tbn, 25 tbc (default)
> > Metadata:
> >   ENCODER : Lavc57.51.100 mpeg4
>
> 57.51.100?
>
> > Stream #0:1: Video: mpeg4 (Simple Profile), none, 320x240 [SAR 1:1
> DAR 4:3], 25 fps, 25 tbr, 1k tbn, 25 tbc (default)
> > Metadata:
> >   ENCODER : Lavc57.51.100 mpeg4
> > Stream #0:2: Attachment: none
> > Metadata:
> >   filename: data
> >   mimetype: test/data
> > Stream #0:3: Attachment: none
> > Metadata:
> >   filename: data
> >   mimetype: test/data
>
> With current git head, I'm getting
>
> Input #0, matroska,webm, from '/tmp/a.mkv':
>   Metadata:
> ENCODER : Lavf57.51.103
>   Duration: N/A, start: 0.00, bitrate: N/A
> Stream #0:0: Video: mpeg4 (Simple Profile), yuv420p, 320x240 [SAR 1:1
> DAR 4:3], 25 fps, 25 tbr, 1k tbn, 25 tbc (default)
> Metadata:
>   ENCODER : Lavc57.61.100 mpeg4
> Stream #0:1: Attachment: none
> Metadata:
>   filename: data
>   mimetype: test/data
>
> After running your test case.


> Are you sure you tested if this behavior was still present in current
> git head before writing this patch? I'm asking because your pasted
> output above reports an old lavc version.
>

Thanks for pointing that out. Indeed, as you say, that symptom is fixed in
head. I thought I had tested in head, although pasted output was admittedly
from an old version (doh).

There may be some value in the patch still, as the SeekHead is not written
in the given case (since the dynamic buffers for crc32 don't wrap all the
header data {seek_head, info, tracks, tags, attachments}, only the
individual elements):
+ Segment, size unknown
|+ EbmlVoid (size: 220)
|+ Segment information

With patch applied, and parsed from mkvinfo:
+ Segment, size unknown
|+ Seek head (subentries will be skipped)
|+ EbmlVoid (size: 149)
|+ Segment information
|+ Segment tracks

And it does allow crc32 to be written in streaming mode:

segment (47,-1) {
  seek_head (59,65) {
crc_32: bytes(4)
...
  info (288,75) {
crc_32: bytes(4)

Not sure if it is worth the additional complexity.


> ___
> 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

[FFmpeg-devel] [PATCH] lavf/matroskaenc.c: use dyn_buf to write header elements for non-seekable outputs.

[Neil Birkbeck]

For non-seekable output files, larger elements written in write_header
(like larger attachments, or possibly many tags) can go over
IO_BUFFER_SIZE meaning seeking back to write valid seek head may fail.

Fate test checksums are the same when "-write_crc32=0". Adding dyn_buf
causes buffer to be seekable and so crc32 is written now (causing the
diffs).

Example to reproduce:

  mkfifo /tmp/a.fifo
  dd if=/dev/zero bs=1024 count=40 > /tmp/data
  ffmpeg -nostats -nostdin -y -f lavfi -i testsrc -vframes 1 \
 -attach /tmp/data -metadata:s:1 mimetype=test/data \
 -f matroska /tmp/a.fifo & cat /tmp/a.fifo > /tmp/a.mkv
  ffprobe /tmp/a.mkv

Previously would generate file like:
 Duration: N/A, start: 0.00, bitrate: N/A
Stream #0:0: Video: mpeg4 (Simple Profile), yuv420p, 320x240 [SAR 1:1 DAR 
4:3], 25 fps, 25 tbr, 1k tbn, 25 tbc (default)
Metadata:
  ENCODER : Lavc57.51.100 mpeg4
Stream #0:1: Video: mpeg4 (Simple Profile), none, 320x240 [SAR 1:1 DAR 
4:3], 25 fps, 25 tbr, 1k tbn, 25 tbc (default)
Metadata:
  ENCODER : Lavc57.51.100 mpeg4
Stream #0:2: Attachment: none
Metadata:
  filename: data
  mimetype: test/data
Stream #0:3: Attachment: none
Metadata:
  filename: data
  mimetype: test/data

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libavformat/matroskaenc.c| 101 +++
 tests/fate/matroska.mak  |  10 -
 tests/fate/wavpack.mak   |   4 +-
 tests/ref/fate/binsub-mksenc |   2 +-
 4 files changed, 75 insertions(+), 42 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 593ddd1..26e2012 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1244,10 +1244,10 @@ static int mkv_write_track(AVFormatContext *s, 
MatroskaMuxContext *mkv,
 return 0;
 }
 
-static int mkv_write_tracks(AVFormatContext *s)
+static int mkv_write_tracks(AVIOContext *pb, AVFormatContext *s)
 {
 MatroskaMuxContext *mkv = s->priv_data;
-AVIOContext *dyn_cp, *pb = s->pb;
+AVIOContext *dyn_cp;
 ebml_master tracks;
 int i, ret, default_stream_exists = 0;
 
@@ -1272,10 +1272,10 @@ static int mkv_write_tracks(AVFormatContext *s)
 return 0;
 }
 
-static int mkv_write_chapters(AVFormatContext *s)
+static int mkv_write_chapters(AVIOContext *pb, AVFormatContext *s)
 {
 MatroskaMuxContext *mkv = s->priv_data;
-AVIOContext *dyn_cp, *pb = s->pb;
+AVIOContext *dyn_cp;
 ebml_master chapters, editionentry;
 AVRational scale = {1, 1E9};
 int i, ret;
@@ -1360,20 +1360,19 @@ static int mkv_write_simpletag(AVIOContext *pb, 
AVDictionaryEntry *t)
 return 0;
 }
 
-static int mkv_write_tag_targets(AVFormatContext *s,
+static int mkv_write_tag_targets(AVIOContext *pb,
+ MatroskaMuxContext *mkv,
  unsigned int elementid, unsigned int uid,
  ebml_master *tags, ebml_master* tag)
 {
-AVIOContext *pb;
-MatroskaMuxContext *mkv = s->priv_data;
 ebml_master targets;
 int ret;
 
 if (!tags->pos) {
-ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_TAGS, 
avio_tell(s->pb));
+ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_TAGS, 
avio_tell(pb));
 if (ret < 0) return ret;
 
-start_ebml_master_crc32(s->pb, >tags_bc, tags, MATROSKA_ID_TAGS, 
0);
+start_ebml_master_crc32(pb, >tags_bc, tags, MATROSKA_ID_TAGS, 0);
 }
 pb = mkv->tags_bc;
 
@@ -1396,15 +1395,15 @@ static int mkv_check_tag_name(const char *name, 
unsigned int elementid)
 av_strcasecmp(name, "language"));
 }
 
-static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, unsigned int 
elementid,
+static int mkv_write_tag(AVIOContext *pb, MatroskaMuxContext *mkv,
+ AVDictionary *m, unsigned int elementid,
  unsigned int uid, ebml_master *tags)
 {
-MatroskaMuxContext *mkv = s->priv_data;
 ebml_master tag;
 int ret;
 AVDictionaryEntry *t = NULL;
 
-ret = mkv_write_tag_targets(s, elementid, uid, tags, );
+ret = mkv_write_tag_targets(pb, mkv, elementid, uid, tags, );
 if (ret < 0)
 return ret;
 
@@ -1431,7 +1430,7 @@ static int mkv_check_tag(AVDictionary *m, unsigned int 
elementid)
 return 0;
 }
 
-static int mkv_write_tags(AVFormatContext *s)
+static int mkv_write_tags(AVIOContext *pb, AVFormatContext *s)
 {
 MatroskaMuxContext *mkv = s->priv_data;
 int i, ret;
@@ -1439,7 +1438,7 @@ static int mkv_write_tags(AVFormatContext *s)
 ff_metadata_conv_ctx(s, ff_mkv_metadata_conv, NULL);
 
 if (mkv_check_tag(s->metadata, 0)) {
-ret = mkv_write_tag(s, s->metadata, 0, 0, >tags);
+ret = mkv_write_tag(pb, mkv, s->me

Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc.c: use metadata key/value to set mastering metadata

[Neil Birkbeck]

The most recent patch should still apply cleanly (unless we have a
better way to set these elements). Thanks

On Tue, Apr 19, 2016 at 8:12 AM, Neil Birkbeck <neil.birkb...@gmail.com> wrote:
> Updated patch attached.
>
> On Sat, Apr 16, 2016 at 7:08 PM, Michael Niedermayer
> <mich...@niedermayer.cc> wrote:
>> On Sun, Apr 03, 2016 at 03:38:33PM -0700, Neil Birkbeck wrote:
>>> Use "master_display" key/value pair to specify mastering metadata in a
>>> similar formatting as accepted by libx265 (unless there is some other
>>> generic way to add side data to a stream from command line).
>>> Currently, the packet side data propagates from an input file to
>>> output file if it is transmuxed (mkv -> mkv). Perhaps we want to also
>>> use this same metadata key in matroskadec to also allow for the
>>> metadata to propagate during transcoding.
>>
>>>  matroskaenc.c |   51 +++
>>>  1 file changed, 47 insertions(+), 4 deletions(-)
>>> d92564f3ec6cf08430a79b64f4d1ec304637afe1  
>>> 0001-lavf-matroskaenc.c-use-metadata-key-value-to-set-mas.patch
>>> From b30d80f6ba4b09811039f64af3e7f709d86df5fe Mon Sep 17 00:00:00 2001
>>> From: Neil Birkbeck <neil.birkb...@gmail.com>
>>> Date: Fri, 1 Apr 2016 17:02:42 -0700
>>> Subject: [PATCH] lavf/matroskaenc.c: use metadata key/value to set mastering
>>>  metadata
>>>
>>> Add key/value metadata interface to allow command line setting of 
>>> AVMasteringDisplayMetadata. The formatting is the same as the option in 
>>> libx265.
>>>
>>> Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
>>
>> this does not apply cleanly anymore after codecpar
>>
>> Applying: lavf/matroskaenc.c: use metadata key/value to set mastering 
>> metadata
>> Using index info to reconstruct a base tree...
>> Falling back to patching base and 3-way merge...
>> Auto-merging libavformat/matroskaenc.c
>> CONFLICT (content): Merge conflict in libavformat/matroskaenc.c
>> Failed to merge in the changes.
>> Patch failed at 0001 lavf/matroskaenc.c: use metadata key/value to set 
>> mastering metadata
>> When you have resolved this problem run "git am --resolved".
>> If you would prefer to skip this patch, instead run "git am --skip".
>> To restore the original branch and stop patching run "git am --abort".
>>
>>
>> [...]
>>
>> --
>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> Democracy is the form of government in which you can choose your dictator
>>
>> ___
>> 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] lavu: Adding hybrid-log gamma enum value and transfer function.

[Neil Birkbeck]

Any other thoughts on we should move forward with the _ARIB_STD name
or to use something like HYBRID_LOG_GAMMA?

On Thu, Apr 21, 2016 at 5:04 PM, Neil Birkbeck <neil.birkb...@gmail.com> wrote:
> Thanks Hendrik.
>
> For now, I've updated the patch with a better comment and commit
> message, and will change name to the more intuitive _HLG or
> (_HYBRID_LOG_GAMMA) if you feel this is better. Seems that the more
> recent enums use a standard as the name for the enum (e.g.,
> SMPTEST2084 is often referred to as Perceptual Quantizer or PQ).
>
>
>
> On Thu, Apr 21, 2016 at 2:03 PM, Hendrik Leppkes <h.lepp...@gmail.com> wrote:
>> On Thu, Apr 21, 2016 at 10:57 PM, Neil Birkbeck <neil.birkb...@gmail.com> 
>> wrote:
>>> The standard:
>>> http://www.arib.or.jp/english/html/overview/doc/2-STD-B67v1_0.pdf
>>>
>>> The choice of enum value of 18 is consistent with HEVC:
>>> http://phenix.it-sudparis.eu/jct/doc_end_user/current_document.php?id=10481
>>>
>>> and also with latest proposal for color trc in mkv:
>>> https://mailarchive.ietf.org/arch/search/?email_list=cellar=1=Colour+Format+proposal
>>>
>>> Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
>>> ---
>>>  libavutil/color_utils.c| 16 
>>>  libavutil/pixdesc.c|  2 +-
>>>  libavutil/pixfmt.h |  1 +
>>>  tests/ref/fate/color_utils | 19 +++
>>>  4 files changed, 37 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavutil/color_utils.c b/libavutil/color_utils.c
>>> index 6dba46a..87e5f5e 100644
>>> --- a/libavutil/color_utils.c
>>> +++ b/libavutil/color_utils.c
>>> @@ -155,6 +155,18 @@ static double avpriv_trc_smpte_st428_1(double Lc)
>>>   :  pow(48.0 * Lc / 52.37, 1.0 / 2.6);
>>>  }
>>>
>>> +
>>> +static double avpriv_trc_arib_std_b67(double Lc) {
>>> +// The function uses the definition from HEVC, which assumes that the 
>>> peak
>>> +// white is input level = 1. (this is equivalent to scaling E = Lc * 
>>> 12 and
>>> +// using the definition from the arib standard)
>>> +const double a = 0.17883277;
>>> +const double b = 0.28466892;
>>> +const double c = 0.55991073;
>>> +return (0.0 > Lc) ? 0.0 :
>>> +(Lc <= 1.0 / 12.0 ? sqrt(3.0 * Lc) : a * log(12.0 * Lc - b) + c);
>>> +}
>>> +
>>>  avpriv_trc_function avpriv_get_trc_function_from_trc(enum 
>>> AVColorTransferCharacteristic trc)
>>>  {
>>>  avpriv_trc_function func = NULL;
>>> @@ -209,6 +221,10 @@ avpriv_trc_function 
>>> avpriv_get_trc_function_from_trc(enum AVColorTransferCharact
>>>  func = avpriv_trc_smpte_st428_1;
>>>  break;
>>>
>>> +case AVCOL_TRC_ARIB_STD_B67:
>>> +func = avpriv_trc_arib_std_b67;
>>> +break;
>>> +
>>>  case AVCOL_TRC_RESERVED0:
>>>  case AVCOL_TRC_UNSPECIFIED:
>>>  case AVCOL_TRC_RESERVED:
>>> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
>>> index 8a9475c..995244b 100644
>>> --- a/libavutil/pixdesc.c
>>> +++ b/libavutil/pixdesc.c
>>> @@ -2080,6 +2080,7 @@ static const char *color_transfer_names[AVCOL_TRC_NB] 
>>> = {
>>>  "bt470bg", "smpte170m", "smpte240m", "linear", "log100",
>>>  "log316", "iec61966-2-4", "bt1361e", "iec61966-2-1",
>>>  "bt2020-10", "bt2020-20", "smpte2084", "smpte428-1",
>>> +"arib-std-b67"
>>>  };
>>>
>>>  static const char *color_space_names[AVCOL_SPC_NB] = {
>>> @@ -2560,4 +2561,3 @@ int main(void){
>>>  }
>>>
>>>  #endif
>>> -
>>> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
>>> index 546eb44..16bd5bc 100644
>>> --- a/libavutil/pixfmt.h
>>> +++ b/libavutil/pixfmt.h
>>> @@ -420,6 +420,7 @@ enum AVColorTransferCharacteristic {
>>>  AVCOL_TRC_BT2020_12= 15, ///< ITU-R BT2020 for 12 bit system
>>>  AVCOL_TRC_SMPTEST2084  = 16, ///< SMPTE ST 2084 for 10, 12, 14 and 16 
>>> bit systems
>>>  AVCOL_TRC_SMPTEST428_1 = 17, ///< SMPTE ST 428-1
>>> +AVCOL_TRC_ARIB_STD_B67 = 18, ///< Hybrid log-gamma
>>
>> I find it odd that the name is seemingly unrelated to t

Re: [FFmpeg-devel] [PATCH] lavu: Adding hybrid-log gamma enum value and transfer function.

[Neil Birkbeck]

Thanks Hendrik.

For now, I've updated the patch with a better comment and commit
message, and will change name to the more intuitive _HLG or
(_HYBRID_LOG_GAMMA) if you feel this is better. Seems that the more
recent enums use a standard as the name for the enum (e.g.,
SMPTEST2084 is often referred to as Perceptual Quantizer or PQ).



On Thu, Apr 21, 2016 at 2:03 PM, Hendrik Leppkes <h.lepp...@gmail.com> wrote:
> On Thu, Apr 21, 2016 at 10:57 PM, Neil Birkbeck <neil.birkb...@gmail.com> 
> wrote:
>> The standard:
>> http://www.arib.or.jp/english/html/overview/doc/2-STD-B67v1_0.pdf
>>
>> The choice of enum value of 18 is consistent with HEVC:
>> http://phenix.it-sudparis.eu/jct/doc_end_user/current_document.php?id=10481
>>
>> and also with latest proposal for color trc in mkv:
>> https://mailarchive.ietf.org/arch/search/?email_list=cellar=1=Colour+Format+proposal
>>
>> Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
>> ---
>>  libavutil/color_utils.c| 16 
>>  libavutil/pixdesc.c|  2 +-
>>  libavutil/pixfmt.h |  1 +
>>  tests/ref/fate/color_utils | 19 +++
>>  4 files changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavutil/color_utils.c b/libavutil/color_utils.c
>> index 6dba46a..87e5f5e 100644
>> --- a/libavutil/color_utils.c
>> +++ b/libavutil/color_utils.c
>> @@ -155,6 +155,18 @@ static double avpriv_trc_smpte_st428_1(double Lc)
>>   :  pow(48.0 * Lc / 52.37, 1.0 / 2.6);
>>  }
>>
>> +
>> +static double avpriv_trc_arib_std_b67(double Lc) {
>> +// The function uses the definition from HEVC, which assumes that the 
>> peak
>> +// white is input level = 1. (this is equivalent to scaling E = Lc * 12 
>> and
>> +// using the definition from the arib standard)
>> +const double a = 0.17883277;
>> +const double b = 0.28466892;
>> +const double c = 0.55991073;
>> +return (0.0 > Lc) ? 0.0 :
>> +(Lc <= 1.0 / 12.0 ? sqrt(3.0 * Lc) : a * log(12.0 * Lc - b) + c);
>> +}
>> +
>>  avpriv_trc_function avpriv_get_trc_function_from_trc(enum 
>> AVColorTransferCharacteristic trc)
>>  {
>>  avpriv_trc_function func = NULL;
>> @@ -209,6 +221,10 @@ avpriv_trc_function 
>> avpriv_get_trc_function_from_trc(enum AVColorTransferCharact
>>  func = avpriv_trc_smpte_st428_1;
>>  break;
>>
>> +case AVCOL_TRC_ARIB_STD_B67:
>> +func = avpriv_trc_arib_std_b67;
>> +break;
>> +
>>  case AVCOL_TRC_RESERVED0:
>>  case AVCOL_TRC_UNSPECIFIED:
>>  case AVCOL_TRC_RESERVED:
>> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
>> index 8a9475c..995244b 100644
>> --- a/libavutil/pixdesc.c
>> +++ b/libavutil/pixdesc.c
>> @@ -2080,6 +2080,7 @@ static const char *color_transfer_names[AVCOL_TRC_NB] 
>> = {
>>  "bt470bg", "smpte170m", "smpte240m", "linear", "log100",
>>  "log316", "iec61966-2-4", "bt1361e", "iec61966-2-1",
>>  "bt2020-10", "bt2020-20", "smpte2084", "smpte428-1",
>> +"arib-std-b67"
>>  };
>>
>>  static const char *color_space_names[AVCOL_SPC_NB] = {
>> @@ -2560,4 +2561,3 @@ int main(void){
>>  }
>>
>>  #endif
>> -
>> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
>> index 546eb44..16bd5bc 100644
>> --- a/libavutil/pixfmt.h
>> +++ b/libavutil/pixfmt.h
>> @@ -420,6 +420,7 @@ enum AVColorTransferCharacteristic {
>>  AVCOL_TRC_BT2020_12= 15, ///< ITU-R BT2020 for 12 bit system
>>  AVCOL_TRC_SMPTEST2084  = 16, ///< SMPTE ST 2084 for 10, 12, 14 and 16 
>> bit systems
>>  AVCOL_TRC_SMPTEST428_1 = 17, ///< SMPTE ST 428-1
>> +AVCOL_TRC_ARIB_STD_B67 = 18, ///< Hybrid log-gamma
>
> I find it odd that the name is seemingly unrelated to the comment, the
> comment should probably elaborate more.
> The commit message could probably also benefit from using the same name.
>
>>  AVCOL_TRC_NB,///< Not part of ABI
>>  };
>>
>> diff --git a/tests/ref/fate/color_utils b/tests/ref/fate/color_utils
>> index 6e80ebd..10f8055 100644
>> --- a/tests/ref/fate/color_utils
>> +++ b/tests/ref/fate/color_utils
>> @@ -283,3 +283,22 @@ AVColorTransferCharacteristic=17 calling 
>> func(15123.456700) expected=39.174525
>>  AVColorTransferCharacteristic

Re: [FFmpeg-devel] [PATCH] Add support for spherical uuid in mov

[Neil Birkbeck]

If you use "spherical-video" and put it in the stream's metadata, it
will more naturally agree with location in stream-level tag in mkv:
https://github.com/google/spatial-media/blob/master/docs/spherical-video-rfc.md

On Wed, Apr 20, 2016 at 6:24 PM, Colin McFadden  wrote:
> ---
>  libavformat/mov.c | 26 ++
>  1 file changed, 26 insertions(+)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 47af98c..2223c81 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -3912,6 +3912,11 @@ static int mov_read_uuid(MOVContext *c, AVIOContext 
> *pb, MOVAtom atom)
>  0xbe, 0x7a, 0xcf, 0xcb, 0x97, 0xa9, 0x42, 0xe8,
>  0x9c, 0x71, 0x99, 0x94, 0x91, 0xe3, 0xaf, 0xac
>  };
> + static const uint8_t uuid_spherical[] = {
> +0xFF, 0xCC, 0x82, 0x63, 0xF8, 0x55, 0x4A, 0x93,
> +0x88, 0x14, 0x58, 0x7A, 0x02, 0x52, 0x1F, 0xDD
> +};
> +
>
>  if (atom.size < sizeof(uuid) || atom.size == INT64_MAX)
>  return AVERROR_INVALIDDATA;
> @@ -3987,6 +3992,27 @@ static int mov_read_uuid(MOVContext *c, AVIOContext 
> *pb, MOVAtom atom)
>  av_dict_set(>fc->metadata, "xmp", buffer, 0);
>  }
>  av_free(buffer);
> +} else if (!memcmp(uuid, uuid_spherical, sizeof(uuid))) {
> +uint8_t *buffer;
> +size_t len = atom.size - sizeof(uuid);
> +
> +buffer = av_mallocz(len + 1);
> +if (!buffer) {
> +return AVERROR(ENOMEM);
> +}
> +ret = avio_read(pb, buffer, len);
> +if (ret < 0) {
> +av_free(buffer);
> +return ret;
> +} else if (ret != len) {
> +av_free(buffer);
> +return AVERROR_INVALIDDATA;
> +}
> +if (c->export_all) {
> +buffer[len] = '\0';
> +av_dict_set(>fc->metadata, "spherical", buffer, 0);
> +}
> +av_free(buffer);
>  }
>  return 0;
>  }
> --
> 2.6.3
>
> ___
> 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] lavf/matroskaenc.c: use metadata key/value to set mastering metadata

[Neil Birkbeck]

Updated patch attached.

On Sat, Apr 16, 2016 at 7:08 PM, Michael Niedermayer
<mich...@niedermayer.cc> wrote:
> On Sun, Apr 03, 2016 at 03:38:33PM -0700, Neil Birkbeck wrote:
>> Use "master_display" key/value pair to specify mastering metadata in a
>> similar formatting as accepted by libx265 (unless there is some other
>> generic way to add side data to a stream from command line).
>> Currently, the packet side data propagates from an input file to
>> output file if it is transmuxed (mkv -> mkv). Perhaps we want to also
>> use this same metadata key in matroskadec to also allow for the
>> metadata to propagate during transcoding.
>
>>  matroskaenc.c |   51 +++
>>  1 file changed, 47 insertions(+), 4 deletions(-)
>> d92564f3ec6cf08430a79b64f4d1ec304637afe1  
>> 0001-lavf-matroskaenc.c-use-metadata-key-value-to-set-mas.patch
>> From b30d80f6ba4b09811039f64af3e7f709d86df5fe Mon Sep 17 00:00:00 2001
>> From: Neil Birkbeck <neil.birkb...@gmail.com>
>> Date: Fri, 1 Apr 2016 17:02:42 -0700
>> Subject: [PATCH] lavf/matroskaenc.c: use metadata key/value to set mastering
>>  metadata
>>
>> Add key/value metadata interface to allow command line setting of 
>> AVMasteringDisplayMetadata. The formatting is the same as the option in 
>> libx265.
>>
>> Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
>
> this does not apply cleanly anymore after codecpar
>
> Applying: lavf/matroskaenc.c: use metadata key/value to set mastering metadata
> Using index info to reconstruct a base tree...
> Falling back to patching base and 3-way merge...
> Auto-merging libavformat/matroskaenc.c
> CONFLICT (content): Merge conflict in libavformat/matroskaenc.c
> Failed to merge in the changes.
> Patch failed at 0001 lavf/matroskaenc.c: use metadata key/value to set 
> mastering metadata
> When you have resolved this problem run "git am --resolved".
> If you would prefer to skip this patch, instead run "git am --skip".
> To restore the original branch and stop patching run "git am --abort".
>
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Democracy is the form of government in which you can choose your dictator
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


0001-lavf-matroskaenc.c-use-metadata-key-value-to-set-mas.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc.c: use metadata key/value to set mastering metadata

[Neil Birkbeck]

We've added support for the correct elements in matroskadec and matroskaenc:
https://github.com/FFmpeg/FFmpeg/commit/e7e5c5e6c4d7e85531bd90a84cbd35c3d84f0453
https://github.com/FFmpeg/FFmpeg/commit/bbda13a7713b8ae8c725c5bb774ac45d614b34eb

The intention of this patch is to expose a way to set this data via
the command-line, but in hindsight I see that wasn't clear in my
description. I'm proposing using the key/value pair on command line to
set the values, e.g.,

 -metadata:s:v:0
master-display="G(13248,34499)B(7500,2999)R(34000,15999)WP(15700,17550)L(1000,100)"

The value is not written in the matroska tags, only to the colour elements.

I'm happy to consider alternatives, but wasn't sure there was a
precedent for instantiating the packet side data from command line.

On Sun, Apr 3, 2016 at 5:13 PM, Dave Rice <d...@dericed.com> wrote:
>
>> On Apr 3, 2016, at 6:38 PM, Neil Birkbeck <neil.birkb...@gmail.com> wrote:
>>
>> Use "master_display" key/value pair to specify mastering metadata in a
>> similar formatting as accepted by libx265 (unless there is some other
>> generic way to add side data to a stream from command line).
>> Currently, the packet side data propagates from an input file to
>> output file if it is transmuxed (mkv -> mkv). Perhaps we want to also
>> use this same metadata key in matroskadec to also allow for the
>> metadata to propagate during transcoding.
>> <0001-lavf-matroskaenc.c-use-metadata-key-value-to-set-mas.patch>___
>
> Wouldn’t it reduce confusion if the proposed MasteringMetadata elements[1] 
> were used rather than rather than storing the same data as Matroska tags?
>
> Dave Rice
>
> [1] as discussed on the CELLAR list 
> https://mailarchive.ietf.org/arch/msg/cellar/hIKLhMdgTMTEwUTeA4ct38h0tmE
> ___
> 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

[FFmpeg-devel] [PATCH] lavf/matroskaenc.c: use metadata key/value to set mastering metadata

[Neil Birkbeck]

Use "master_display" key/value pair to specify mastering metadata in a
similar formatting as accepted by libx265 (unless there is some other
generic way to add side data to a stream from command line).
Currently, the packet side data propagates from an input file to
output file if it is transmuxed (mkv -> mkv). Perhaps we want to also
use this same metadata key in matroskadec to also allow for the
metadata to propagate during transcoding.
From b30d80f6ba4b09811039f64af3e7f709d86df5fe Mon Sep 17 00:00:00 2001
From: Neil Birkbeck <neil.birkb...@gmail.com>
Date: Fri, 1 Apr 2016 17:02:42 -0700
Subject: [PATCH] lavf/matroskaenc.c: use metadata key/value to set mastering
 metadata

Add key/value metadata interface to allow command line setting of AVMasteringDisplayMetadata. The formatting is the same as the option in libx265.

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libavformat/matroskaenc.c | 51 +++
 1 file changed, 47 insertions(+), 4 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 0546686..30ff714 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -732,10 +732,54 @@ static int mkv_write_codecprivate(AVFormatContext *s, AVIOContext *pb,
 }
 
 static int mkv_write_video_color(AVIOContext *pb, AVCodecContext *codec, AVStream *st) {
+AVDictionaryEntry *tag;
 int side_data_size = 0;
-const uint8_t *side_data = av_stream_get_side_data(
+uint8_t *side_data = av_stream_get_side_data(
 st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA, _data_size);
 ebml_master colorinfo = start_ebml_master(pb, MATROSKA_ID_VIDEOCOLOR, 0);
+AVMasteringDisplayMetadata *metadata =
+(side_data_size == sizeof(AVMasteringDisplayMetadata)) ?
+(AVMasteringDisplayMetadata*)side_data : NULL;
+
+// If key-value pair metadata is specified, override the packet side data
+// Accept a format similar to the command line argument in x265:
+//G(x,y)B(x,y)R(x,y)WP(x,y),L(max,min)
+if (tag = av_dict_get(st->metadata, "master_display", NULL, 0)) {
+int primaries[4][2];
+int luma[2];
+int num_read = sscanf(tag->value,
+  "G(%d,%d)B(%d,%d)R(%d,%d)WP(%d,%d)L(%d,%d)",
+  primaries[1], primaries[1] + 1,
+  primaries[2], primaries[2] + 1,
+  primaries[0], primaries[0] + 1,
+  primaries[3], primaries[3] + 1,
+  luma + 1, luma + 0);
+if (num_read >= 8) {
+const float chroma_denom = 5.f;
+const float luma_denom = 1.f;
+int i, j;
+if (!metadata) {
+metadata = (AVMasteringDisplayMetadata*) av_stream_new_side_data(
+st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
+sizeof(AVMasteringDisplayMetadata));
+memset(metadata, 0, sizeof(AVMasteringDisplayMetadata));
+}
+for (i = 0; i < 3; ++i) {
+for (j = 0; j < 2; ++j) {
+metadata->display_primaries[i][j] =
+av_make_q(primaries[i][j], chroma_denom);
+}
+}
+metadata->white_point[0] = av_make_q(primaries[3][0], chroma_denom);
+metadata->white_point[1] = av_make_q(primaries[3][1], chroma_denom);
+metadata->has_primaries = 1;
+if (num_read == 10) {
+metadata->min_luminance = av_make_q(luma[0], luma_denom);
+metadata->max_luminance = av_make_q(luma[1], luma_denom);
+metadata->has_luminance = 1;
+}
+}
+}
 
 if (codec->color_trc != AVCOL_TRC_UNSPECIFIED &&
 codec->color_trc < AVCOL_TRC_NB) {
@@ -754,11 +798,9 @@ static int mkv_write_video_color(AVIOContext *pb, AVCodecContext *codec, AVStrea
 codec->color_range < AVCOL_RANGE_NB) {
 put_ebml_uint(pb, MATROSKA_ID_VIDEOCOLORRANGE, codec->color_range);
 }
-if (side_data_size == sizeof(AVMasteringDisplayMetadata)) {
+if (metadata) {
 ebml_master meta_element = start_ebml_master(
 pb, MATROSKA_ID_VIDEOCOLORMASTERINGMETA, 0);
-const AVMasteringDisplayMetadata *metadata =
-(const AVMasteringDisplayMetadata*)side_data;
 if (metadata->has_primaries) {
 put_ebml_float(pb, MATROSKA_ID_VIDEOCOLOR_RX,
av_q2d(metadata->display_primaries[0][0]));
@@ -1278,6 +1320,7 @@ static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, unsigned int eleme
 av_strcasecmp(t->key, "stereo_mode") &&
 av_strcasecmp(t->key, "creation_time") &&
 a

[FFmpeg-devel] [PATCH] lavf/matroskaenc.c: add early support for colour elements

[Neil Birkbeck]

Adding early support for a subset of the proposed colour elements
according to the latest version of spec:
https://mailarchive.ietf.org/arch/search/?email_list=cellar=1=hIKLhMdgTMTEwUTeA4ct38h0tmE

Like matroskadec, I've left out elements for pix_fmt related things
 as there still seems to be some discussion around these.

The new elements are exposed under strict experimental mode.

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libavformat/matroskaenc.c | 63 ++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index f42434b..ce5c605 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -41,6 +41,7 @@
 #include "libavutil/intfloat.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/lfg.h"
+#include "libavutil/mastering_display_metadata.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/opt.h"
 #include "libavutil/random_seed.h"
@@ -730,6 +731,64 @@ static int mkv_write_codecprivate(AVFormatContext *s, 
AVIOContext *pb,
 return ret;
 }
 
+static int mkv_write_video_color(AVIOContext *pb, AVCodecContext *codec, 
AVStream *st) {
+int side_data_size = 0;
+const uint8_t *side_data = av_stream_get_side_data(
+st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA, _data_size);
+ebml_master colorinfo = start_ebml_master(pb, MATROSKA_ID_VIDEOCOLOR, 0);
+
+if (codec->color_trc != AVCOL_TRC_UNSPECIFIED &&
+codec->color_trc < AVCOL_TRC_NB) {
+put_ebml_uint(pb, MATROSKA_ID_VIDEOCOLORTRANSFERCHARACTERISTICS,
+  codec->color_trc);
+}
+if (codec->colorspace != AVCOL_SPC_UNSPECIFIED &&
+codec->colorspace < AVCOL_SPC_NB) {
+put_ebml_uint(pb, MATROSKA_ID_VIDEOCOLORMATRIXCOEFF, 
codec->colorspace);
+}
+if (codec->color_primaries != AVCOL_PRI_UNSPECIFIED &&
+codec->color_primaries < AVCOL_PRI_NB) {
+put_ebml_uint(pb, MATROSKA_ID_VIDEOCOLORPRIMARIES, 
codec->color_primaries);
+}
+if (codec->color_range != AVCOL_RANGE_UNSPECIFIED &&
+codec->color_range < AVCOL_RANGE_NB) {
+put_ebml_uint(pb, MATROSKA_ID_VIDEOCOLORRANGE, codec->color_range);
+}
+if (side_data_size == sizeof(AVMasteringDisplayMetadata)) {
+ebml_master meta_element = start_ebml_master(
+pb, MATROSKA_ID_VIDEOCOLORMASTERINGMETA, 0);
+const AVMasteringDisplayMetadata *metadata =
+(const AVMasteringDisplayMetadata*)side_data;
+if (metadata->has_primaries) {
+put_ebml_float(pb, MATROSKA_ID_VIDEOCOLOR_RX,
+   av_q2d(metadata->display_primaries[0][0]));
+put_ebml_float(pb, MATROSKA_ID_VIDEOCOLOR_RY,
+   av_q2d(metadata->display_primaries[0][1]));
+put_ebml_float(pb, MATROSKA_ID_VIDEOCOLOR_GX,
+   av_q2d(metadata->display_primaries[1][0]));
+put_ebml_float(pb, MATROSKA_ID_VIDEOCOLOR_GY,
+   av_q2d(metadata->display_primaries[1][1]));
+put_ebml_float(pb, MATROSKA_ID_VIDEOCOLOR_BX,
+   av_q2d(metadata->display_primaries[2][0]));
+put_ebml_float(pb, MATROSKA_ID_VIDEOCOLOR_BY,
+   av_q2d(metadata->display_primaries[2][1]));
+put_ebml_float(pb, MATROSKA_ID_VIDEOCOLOR_WHITEX,
+   av_q2d(metadata->white_point[0]));
+put_ebml_float(pb, MATROSKA_ID_VIDEOCOLOR_WHITEY,
+   av_q2d(metadata->white_point[1]));
+}
+if (metadata->has_luminance) {
+put_ebml_float(pb, MATROSKA_ID_VIDEOCOLOR_LUMINANCEMAX,
+   av_q2d(metadata->max_luminance));
+put_ebml_float(pb, MATROSKA_ID_VIDEOCOLOR_LUMINANCEMIN,
+   av_q2d(metadata->min_luminance));
+}
+end_ebml_master(pb, meta_element);
+}
+end_ebml_master(pb, colorinfo);
+return 0;
+}
+
 
 static int mkv_write_stereo_mode(AVFormatContext *s, AVIOContext *pb,
  AVStream *st, int mode, int *h_width, int 
*h_height)
@@ -1015,7 +1074,9 @@ static int mkv_write_track(AVFormatContext *s, 
MatroskaMuxContext *mkv,
 uint32_t color_space = av_le2ne32(codec->codec_tag);
 put_ebml_binary(pb, MATROSKA_ID_VIDEOCOLORSPACE, _space, 
sizeof(color_space));
 }
-
+if (s->strict_std_compliance <= FF_COMPLIANCE_UNOFFICIAL) {
+mkv_write_video_color(pb, codec, st);
+}
 end_ebml_master(pb, subinfo);
 break;
 
-- 
2.7.0.rc3.207.g0ac5344

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

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: Add early support for colour elements

[Neil Birkbeck]

And yet another revision, where the syntax lists are actually terminated...

On Thu, Mar 3, 2016 at 12:09 PM, Neil Birkbeck <neil.birkb...@gmail.com> wrote:
> I've attached a slightly cleaner (more isolated) patch that moves the
> parsing into a helper function.
>
> On Mon, Feb 29, 2016 at 5:15 PM, Neil Birkbeck <neil.birkb...@gmail.com> 
> wrote:
>> Adding early support for a subset of the proposed colour elements
>> according to the latest version of spec:
>> https://mailarchive.ietf.org/arch/search/?email_list=cellar=1=hIKLhMdgTMTEwUTeA4ct38h0tmE
>>
>> I've left out elements for pix_fmt related things as there still
>> seems to be some discussion around these, and the max_cll/max_fall
>> are currently not propagated as there is not yet side data for them.
>>
>> The new elements are exposed under strict experimental mode.
>>
>> Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
>> ---
>>  libavformat/matroska.h|  28 ++
>>  libavformat/matroskadec.c | 136 
>> ++
>>  2 files changed, 164 insertions(+)
>>
>> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
>> index a654e0c..e97fe6b 100644
>> --- a/libavformat/matroska.h
>> +++ b/libavformat/matroska.h
>> @@ -123,6 +123,34 @@
>>  #define MATROSKA_ID_VIDEOALPHAMODE 0x53C0
>>  #define MATROSKA_ID_VIDEOASPECTRATIO 0x54B3
>>  #define MATROSKA_ID_VIDEOCOLORSPACE 0x2EB524
>> +#define MATROSKA_ID_VIDEOCOLOR 0x55B0
>> +
>> +#define MATROSKA_ID_VIDEOCOLORMATRIXCOEFF 0x55B1
>> +#define MATROSKA_ID_VIDEOCOLORBITSPERCHANNEL 0x55B2
>> +#define MATROSKA_ID_VIDEOCOLORCHROMASUBHORZ 0x55B3
>> +#define MATROSKA_ID_VIDEOCOLORCHROMASUBVERT 0x55B4
>> +#define MATROSKA_ID_VIDEOCOLORCBSUBHORZ 0x55B5
>> +#define MATROSKA_ID_VIDEOCOLORCBSUBVERT 0x55B6
>> +#define MATROSKA_ID_VIDEOCOLORCHROMASITINGHORZ 0x55B7
>> +#define MATROSKA_ID_VIDEOCOLORCHROMASITINGVERT 0x55B8
>> +#define MATROSKA_ID_VIDEOCOLORRANGE 0x55B9
>> +#define MATROSKA_ID_VIDEOCOLORTRANSFERCHARACTERISTICS 0x55BA
>> +
>> +#define MATROSKA_ID_VIDEOCOLORPRIMARIES 0x55BB
>> +#define MATROSKA_ID_VIDEOCOLORMAXCLL 0x55BC
>> +#define MATROSKA_ID_VIDEOCOLORMAXFALL 0x55BD
>> +
>> +#define MATROSKA_ID_VIDEOCOLORMASTERINGMETA 0x55D0
>> +#define MATROSKA_ID_VIDEOCOLOR_RX 0x55D1
>> +#define MATROSKA_ID_VIDEOCOLOR_RY 0x55D2
>> +#define MATROSKA_ID_VIDEOCOLOR_GX 0x55D3
>> +#define MATROSKA_ID_VIDEOCOLOR_GY 0x55D4
>> +#define MATROSKA_ID_VIDEOCOLOR_BX 0x55D5
>> +#define MATROSKA_ID_VIDEOCOLOR_BY 0x55D6
>> +#define MATROSKA_ID_VIDEOCOLOR_WHITEX 0x55D7
>> +#define MATROSKA_ID_VIDEOCOLOR_WHITEY 0x55D8
>> +#define MATROSKA_ID_VIDEOCOLOR_LUMINANCEMAX 0x55D9
>> +#define MATROSKA_ID_VIDEOCOLOR_LUMINANCEMIN 0x55DA
>>
>>  /* IDs in the trackaudio master */
>>  #define MATROSKA_ID_AUDIOSAMPLINGFREQ 0xB5
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index d20568c..4510e8e 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -39,6 +39,7 @@
>>  #include "libavutil/intfloat.h"
>>  #include "libavutil/intreadwrite.h"
>>  #include "libavutil/lzo.h"
>> +#include "libavutil/mastering_display_metadata.h"
>>  #include "libavutil/mathematics.h"
>>  #include "libavutil/opt.h"
>>  #include "libavutil/time_internal.h"
>> @@ -130,6 +131,36 @@ typedef struct MatroskaTrackEncoding {
>>  MatroskaTrackEncryption encryption;
>>  } MatroskaTrackEncoding;
>>
>> +typedef struct MatroskaMasteringMeta {
>> +double r_x;
>> +double r_y;
>> +double g_x;
>> +double g_y;
>> +double b_x;
>> +double b_y;
>> +double white_x;
>> +double white_y;
>> +double max_luminance;
>> +double min_luminance;
>> +} MatroskaMasteringMeta;
>> +
>> +typedef struct MatroskaTrackVideoColor {
>> +uint64_t matrix_coefficients;
>> +uint64_t bits_per_channel;
>> +uint64_t chroma_sub_horz;
>> +uint64_t chroma_sub_vert;
>> +uint64_t cb_sub_horz;
>> +uint64_t cb_sub_vert;
>> +uint64_t chroma_siting_horz;
>> +uint64_t chroma_siting_vert;
>> +uint64_t range;
>> +uint64_t transfer_characteristics;
>> +uint64_t primaries;
>> +uint64_t max_cll;
>> +uint64_t max_fall;
>> +MatroskaMasteringMeta mastering_meta;
>> +} MatroskaTrackVideoColor;
>> +

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: Add early support for colour elements

[Neil Birkbeck]

I've attached a slightly cleaner (more isolated) patch that moves the
parsing into a helper function.

On Mon, Feb 29, 2016 at 5:15 PM, Neil Birkbeck <neil.birkb...@gmail.com> wrote:
> Adding early support for a subset of the proposed colour elements
> according to the latest version of spec:
> https://mailarchive.ietf.org/arch/search/?email_list=cellar=1=hIKLhMdgTMTEwUTeA4ct38h0tmE
>
> I've left out elements for pix_fmt related things as there still
> seems to be some discussion around these, and the max_cll/max_fall
> are currently not propagated as there is not yet side data for them.
>
> The new elements are exposed under strict experimental mode.
>
> Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
> ---
>  libavformat/matroska.h|  28 ++
>  libavformat/matroskadec.c | 136 
> ++
>  2 files changed, 164 insertions(+)
>
> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
> index a654e0c..e97fe6b 100644
> --- a/libavformat/matroska.h
> +++ b/libavformat/matroska.h
> @@ -123,6 +123,34 @@
>  #define MATROSKA_ID_VIDEOALPHAMODE 0x53C0
>  #define MATROSKA_ID_VIDEOASPECTRATIO 0x54B3
>  #define MATROSKA_ID_VIDEOCOLORSPACE 0x2EB524
> +#define MATROSKA_ID_VIDEOCOLOR 0x55B0
> +
> +#define MATROSKA_ID_VIDEOCOLORMATRIXCOEFF 0x55B1
> +#define MATROSKA_ID_VIDEOCOLORBITSPERCHANNEL 0x55B2
> +#define MATROSKA_ID_VIDEOCOLORCHROMASUBHORZ 0x55B3
> +#define MATROSKA_ID_VIDEOCOLORCHROMASUBVERT 0x55B4
> +#define MATROSKA_ID_VIDEOCOLORCBSUBHORZ 0x55B5
> +#define MATROSKA_ID_VIDEOCOLORCBSUBVERT 0x55B6
> +#define MATROSKA_ID_VIDEOCOLORCHROMASITINGHORZ 0x55B7
> +#define MATROSKA_ID_VIDEOCOLORCHROMASITINGVERT 0x55B8
> +#define MATROSKA_ID_VIDEOCOLORRANGE 0x55B9
> +#define MATROSKA_ID_VIDEOCOLORTRANSFERCHARACTERISTICS 0x55BA
> +
> +#define MATROSKA_ID_VIDEOCOLORPRIMARIES 0x55BB
> +#define MATROSKA_ID_VIDEOCOLORMAXCLL 0x55BC
> +#define MATROSKA_ID_VIDEOCOLORMAXFALL 0x55BD
> +
> +#define MATROSKA_ID_VIDEOCOLORMASTERINGMETA 0x55D0
> +#define MATROSKA_ID_VIDEOCOLOR_RX 0x55D1
> +#define MATROSKA_ID_VIDEOCOLOR_RY 0x55D2
> +#define MATROSKA_ID_VIDEOCOLOR_GX 0x55D3
> +#define MATROSKA_ID_VIDEOCOLOR_GY 0x55D4
> +#define MATROSKA_ID_VIDEOCOLOR_BX 0x55D5
> +#define MATROSKA_ID_VIDEOCOLOR_BY 0x55D6
> +#define MATROSKA_ID_VIDEOCOLOR_WHITEX 0x55D7
> +#define MATROSKA_ID_VIDEOCOLOR_WHITEY 0x55D8
> +#define MATROSKA_ID_VIDEOCOLOR_LUMINANCEMAX 0x55D9
> +#define MATROSKA_ID_VIDEOCOLOR_LUMINANCEMIN 0x55DA
>
>  /* IDs in the trackaudio master */
>  #define MATROSKA_ID_AUDIOSAMPLINGFREQ 0xB5
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index d20568c..4510e8e 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -39,6 +39,7 @@
>  #include "libavutil/intfloat.h"
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/lzo.h"
> +#include "libavutil/mastering_display_metadata.h"
>  #include "libavutil/mathematics.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/time_internal.h"
> @@ -130,6 +131,36 @@ typedef struct MatroskaTrackEncoding {
>  MatroskaTrackEncryption encryption;
>  } MatroskaTrackEncoding;
>
> +typedef struct MatroskaMasteringMeta {
> +double r_x;
> +double r_y;
> +double g_x;
> +double g_y;
> +double b_x;
> +double b_y;
> +double white_x;
> +double white_y;
> +double max_luminance;
> +double min_luminance;
> +} MatroskaMasteringMeta;
> +
> +typedef struct MatroskaTrackVideoColor {
> +uint64_t matrix_coefficients;
> +uint64_t bits_per_channel;
> +uint64_t chroma_sub_horz;
> +uint64_t chroma_sub_vert;
> +uint64_t cb_sub_horz;
> +uint64_t cb_sub_vert;
> +uint64_t chroma_siting_horz;
> +uint64_t chroma_siting_vert;
> +uint64_t range;
> +uint64_t transfer_characteristics;
> +uint64_t primaries;
> +uint64_t max_cll;
> +uint64_t max_fall;
> +MatroskaMasteringMeta mastering_meta;
> +} MatroskaTrackVideoColor;
> +
>  typedef struct MatroskaTrackVideo {
>  double   frame_rate;
>  uint64_t display_width;
> @@ -139,6 +170,7 @@ typedef struct MatroskaTrackVideo {
>  EbmlBin color_space;
>  uint64_t stereo_mode;
>  uint64_t alpha_mode;
> +MatroskaTrackVideoColor color;
>  } MatroskaTrackVideo;
>
>  typedef struct MatroskaTrackAudio {
> @@ -356,6 +388,36 @@ static const EbmlSyntax matroska_info[] = {
>  { 0 }
>  };
>
> +static const EbmlSyntax matroska_mastering_meta[] = {
> +{ MATROSKA_ID_VIDEOCOLOR_RX, EBML_FLOAT, 0, 
> offsetof(MatroskaMaster

[FFmpeg-devel] [PATCH] lavf/dump.c: Print mastering display metadata

[Neil Birkbeck]

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libavformat/dump.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/libavformat/dump.c b/libavformat/dump.c
index 56c285d..9e7c12b 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -26,6 +26,7 @@
 #include "libavutil/display.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/log.h"
+#include "libavutil/mastering_display_metadata.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/opt.h"
 #include "libavutil/avstring.h"
@@ -352,6 +353,23 @@ static void dump_cpb(void *ctx, AVPacketSideData *sd)
cpb->vbv_delay);
 }
 
+static void dump_mastering_display_metadata(void *ctx, AVPacketSideData* sd) {
+AVMasteringDisplayMetadata* metadata = 
(AVMasteringDisplayMetadata*)sd->data;
+av_log(ctx, AV_LOG_INFO, "Mastering Display Metadata, "
+   "has_primaries:%d has_luminance:%d "
+   "r(%5.4f,%5.4f) g(%5.4f,%5.4f) b(%5.4f %5.4f) wp(%5.4f, %5.4f) "
+   "min_luminance=%f, max_luminance=%f\n",
+   metadata->has_primaries, metadata->has_luminance,
+   av_q2d(metadata->display_primaries[0][0]),
+   av_q2d(metadata->display_primaries[0][1]),
+   av_q2d(metadata->display_primaries[1][0]),
+   av_q2d(metadata->display_primaries[1][1]),
+   av_q2d(metadata->display_primaries[2][0]),
+   av_q2d(metadata->display_primaries[2][1]),
+   av_q2d(metadata->white_point[0]), av_q2d(metadata->white_point[1]),
+   av_q2d(metadata->min_luminance), av_q2d(metadata->max_luminance));
+}
+
 static void dump_sidedata(void *ctx, AVStream *st, const char *indent)
 {
 int i;
@@ -400,6 +418,9 @@ static void dump_sidedata(void *ctx, AVStream *st, const 
char *indent)
 av_log(ctx, AV_LOG_INFO, "cpb: ");
 dump_cpb(ctx, );
 break;
+case AV_PKT_DATA_MASTERING_DISPLAY_METADATA:
+dump_mastering_display_metadata(ctx, );
+break;
 default:
 av_log(ctx, AV_LOG_WARNING,
"unknown side data type %d (%d bytes)", sd.type, sd.size);
-- 
2.7.0.rc3.207.g0ac5344

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

[FFmpeg-devel] [PATCH] lavf/matroskadec: Add early support for colour elements

[Neil Birkbeck]

Adding early support for a subset of the proposed colour elements
according to the latest version of spec:
https://mailarchive.ietf.org/arch/search/?email_list=cellar=1=hIKLhMdgTMTEwUTeA4ct38h0tmE

I've left out elements for pix_fmt related things as there still
seems to be some discussion around these, and the max_cll/max_fall
are currently not propagated as there is not yet side data for them.

The new elements are exposed under strict experimental mode.

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libavformat/matroska.h|  28 ++
 libavformat/matroskadec.c | 136 ++
 2 files changed, 164 insertions(+)

diff --git a/libavformat/matroska.h b/libavformat/matroska.h
index a654e0c..e97fe6b 100644
--- a/libavformat/matroska.h
+++ b/libavformat/matroska.h
@@ -123,6 +123,34 @@
 #define MATROSKA_ID_VIDEOALPHAMODE 0x53C0
 #define MATROSKA_ID_VIDEOASPECTRATIO 0x54B3
 #define MATROSKA_ID_VIDEOCOLORSPACE 0x2EB524
+#define MATROSKA_ID_VIDEOCOLOR 0x55B0
+
+#define MATROSKA_ID_VIDEOCOLORMATRIXCOEFF 0x55B1
+#define MATROSKA_ID_VIDEOCOLORBITSPERCHANNEL 0x55B2
+#define MATROSKA_ID_VIDEOCOLORCHROMASUBHORZ 0x55B3
+#define MATROSKA_ID_VIDEOCOLORCHROMASUBVERT 0x55B4
+#define MATROSKA_ID_VIDEOCOLORCBSUBHORZ 0x55B5
+#define MATROSKA_ID_VIDEOCOLORCBSUBVERT 0x55B6
+#define MATROSKA_ID_VIDEOCOLORCHROMASITINGHORZ 0x55B7
+#define MATROSKA_ID_VIDEOCOLORCHROMASITINGVERT 0x55B8
+#define MATROSKA_ID_VIDEOCOLORRANGE 0x55B9
+#define MATROSKA_ID_VIDEOCOLORTRANSFERCHARACTERISTICS 0x55BA
+
+#define MATROSKA_ID_VIDEOCOLORPRIMARIES 0x55BB
+#define MATROSKA_ID_VIDEOCOLORMAXCLL 0x55BC
+#define MATROSKA_ID_VIDEOCOLORMAXFALL 0x55BD
+
+#define MATROSKA_ID_VIDEOCOLORMASTERINGMETA 0x55D0
+#define MATROSKA_ID_VIDEOCOLOR_RX 0x55D1
+#define MATROSKA_ID_VIDEOCOLOR_RY 0x55D2
+#define MATROSKA_ID_VIDEOCOLOR_GX 0x55D3
+#define MATROSKA_ID_VIDEOCOLOR_GY 0x55D4
+#define MATROSKA_ID_VIDEOCOLOR_BX 0x55D5
+#define MATROSKA_ID_VIDEOCOLOR_BY 0x55D6
+#define MATROSKA_ID_VIDEOCOLOR_WHITEX 0x55D7
+#define MATROSKA_ID_VIDEOCOLOR_WHITEY 0x55D8
+#define MATROSKA_ID_VIDEOCOLOR_LUMINANCEMAX 0x55D9
+#define MATROSKA_ID_VIDEOCOLOR_LUMINANCEMIN 0x55DA
 
 /* IDs in the trackaudio master */
 #define MATROSKA_ID_AUDIOSAMPLINGFREQ 0xB5
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index d20568c..4510e8e 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -39,6 +39,7 @@
 #include "libavutil/intfloat.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/lzo.h"
+#include "libavutil/mastering_display_metadata.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/opt.h"
 #include "libavutil/time_internal.h"
@@ -130,6 +131,36 @@ typedef struct MatroskaTrackEncoding {
 MatroskaTrackEncryption encryption;
 } MatroskaTrackEncoding;
 
+typedef struct MatroskaMasteringMeta {
+double r_x;
+double r_y;
+double g_x;
+double g_y;
+double b_x;
+double b_y;
+double white_x;
+double white_y;
+double max_luminance;
+double min_luminance;
+} MatroskaMasteringMeta;
+
+typedef struct MatroskaTrackVideoColor {
+uint64_t matrix_coefficients;
+uint64_t bits_per_channel;
+uint64_t chroma_sub_horz;
+uint64_t chroma_sub_vert;
+uint64_t cb_sub_horz;
+uint64_t cb_sub_vert;
+uint64_t chroma_siting_horz;
+uint64_t chroma_siting_vert;
+uint64_t range;
+uint64_t transfer_characteristics;
+uint64_t primaries;
+uint64_t max_cll;
+uint64_t max_fall;
+MatroskaMasteringMeta mastering_meta;
+} MatroskaTrackVideoColor;
+
 typedef struct MatroskaTrackVideo {
 double   frame_rate;
 uint64_t display_width;
@@ -139,6 +170,7 @@ typedef struct MatroskaTrackVideo {
 EbmlBin color_space;
 uint64_t stereo_mode;
 uint64_t alpha_mode;
+MatroskaTrackVideoColor color;
 } MatroskaTrackVideo;
 
 typedef struct MatroskaTrackAudio {
@@ -356,6 +388,36 @@ static const EbmlSyntax matroska_info[] = {
 { 0 }
 };
 
+static const EbmlSyntax matroska_mastering_meta[] = {
+{ MATROSKA_ID_VIDEOCOLOR_RX, EBML_FLOAT, 0, 
offsetof(MatroskaMasteringMeta, r_x), { .f=-1 } },
+{ MATROSKA_ID_VIDEOCOLOR_RY, EBML_FLOAT, 0, 
offsetof(MatroskaMasteringMeta, r_y), { .f=-1 } },
+{ MATROSKA_ID_VIDEOCOLOR_GX, EBML_FLOAT, 0, 
offsetof(MatroskaMasteringMeta, g_x), { .f=-1 } },
+{ MATROSKA_ID_VIDEOCOLOR_GY, EBML_FLOAT, 0, 
offsetof(MatroskaMasteringMeta, g_y), { .f=-1 } },
+{ MATROSKA_ID_VIDEOCOLOR_BX, EBML_FLOAT, 0, 
offsetof(MatroskaMasteringMeta, b_x), { .f=-1 } },
+{ MATROSKA_ID_VIDEOCOLOR_BY, EBML_FLOAT, 0, 
offsetof(MatroskaMasteringMeta, b_y), { .f=-1 } },
+{ MATROSKA_ID_VIDEOCOLOR_WHITEX, EBML_FLOAT, 0, 
offsetof(MatroskaMasteringMeta, white_x), { .f=-1 } },
+{ MATROSKA_ID_VIDEOCOLOR_WHITEY, EBML_FLOAT, 0, 
offsetof(MatroskaMasteringMeta, white_y), { .f=-1 } },
+{ MATROSKA_ID_VIDEOCOLOR_LUMINANCE

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

[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 <h.lepp...@gmail.com> wrote:
> On Fri, Feb 5, 2016 at 3:05 AM, Neil Birkbeck <neil.birkb...@gmail.com> 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.

[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
<mich...@niedermayer.cc> 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 <neil.birkb...@gmail.com> 
>> 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 <h.lepp...@gmail.com>
>> > wrote:
>> >>
>> >> On Mon, Jan 25, 2016 at 10:37 PM, Michael Niedermayer
>> >> <mich...@niedermayer.cc> 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 <neil.birkb...@gmail.com>
>> 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 si

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

[Neil Birkbeck]

Please see updated patch.

On Mon, Jan 25, 2016 at 11:39 PM, Neil Birkbeck <neil.birkb...@gmail.com> 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 <h.lepp...@gmail.com>
> wrote:
>>
>> On Mon, Jan 25, 2016 at 10:37 PM, Michael Niedermayer
>> <mich...@niedermayer.cc> 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 <neil.birkb...@gmail.com>
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 <neil.birkb...@gmail.com>
---
 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->

[FFmpeg-devel] [PATCH] libavutil/mastering_display_metadata.h: change fields to be rationals as this is how they are typically coded.

[Neil Birkbeck]

From: "Vittorio Gambaletta (VittGam)" <ffmpeg-...@vittgam.net>

(this structure is not referenced anywhere yet)

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libavutil/mastering_display_metadata.h | 10 ++
 libavutil/version.h|  3 +--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/libavutil/mastering_display_metadata.h 
b/libavutil/mastering_display_metadata.h
index 781c1fd..936533f 100644
--- a/libavutil/mastering_display_metadata.h
+++ b/libavutil/mastering_display_metadata.h
@@ -22,6 +22,8 @@
 #define AVUTIL_MASTERING_DISPLAY_METADATA_H
 
 #include "frame.h"
+#include "rational.h"
+
 
 /**
  * Mastering display metadata capable of representing the color volume of
@@ -37,22 +39,22 @@ typedef struct AVMasteringDisplayMetadata {
 /**
  * CIE 1931 xy chromaticity coords of color primaries (r, g, b order).
  */
-float display_primaries[3][2];
+AVRational display_primaries[3][2];
 
 /**
  * CIE 1931 xy chromaticity coords of white point.
  */
-float white_point[2];
+AVRational white_point[2];
 
 /**
  * Min luminance of mastering display (cd/m^2).
  */
-float min_luminance;
+AVRational min_luminance;
 
 /**
  * Max luminance of mastering display (cd/m^2).
  */
-float max_luminance;
+AVRational max_luminance;
 
 /**
  * Flag indicating whether the display primaries (and white point) are set.
diff --git a/libavutil/version.h b/libavutil/version.h
index 42f7cde..d63696d 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -65,7 +65,7 @@
 
 #define LIBAVUTIL_VERSION_MAJOR  55
 #define LIBAVUTIL_VERSION_MINOR  15
-#define LIBAVUTIL_VERSION_MICRO 100
+#define LIBAVUTIL_VERSION_MICRO 101
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
LIBAVUTIL_VERSION_MINOR, \
@@ -126,4 +126,3 @@
  */
 
 #endif /* AVUTIL_VERSION_H */
-
-- 
2.7.0.rc3.207.g0ac5344

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

Re: [FFmpeg-devel] [PATCH] libavutil/mastering_display_metadata.h: change fields to be rationals as this is how they are typically coded.

[Neil Birkbeck]

Some sort of squash fail. Apologies.

On Tue, Jan 26, 2016 at 1:13 PM, Vittorio Gambaletta (VittGam) <
ffmpeg-...@vittgam.net> wrote:

> Hi,
>
> On 26/01/2016 22:09:02 CET, Neil Birkbeck wrote:
>
>> From: "Vittorio Gambaletta (VittGam)" <ffmpeg-...@vittgam.net>
>>
>
> This is not from me...
>
> Cheers,
> Vittorio
> ___
> 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] libavutil/mastering_display_metadata.h: change fields to be rationals as this is how they are typically coded.

[Neil Birkbeck]

Patch with corrected "From:" is attached.

On Tue, Jan 26, 2016 at 2:28 PM, Vittorio Gambaletta (VittGam)
<ffmpeg-...@vittgam.net> wrote:
> On 26/01/2016 23:05:22 CET, Neil Birkbeck wrote:
>>
>> Some sort of squash fail. Apologies.
>
>
> No problem!
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
From f6c9ccc0899c10faffb98b224affc081a523b1ca Mon Sep 17 00:00:00 2001
From: Neil Birkbeck <neil.birkb...@gmail.com>
Date: Tue, 26 Jan 2016 14:14:20 -0800
Subject: [PATCH] libavutil/mastering_display_metadata.h: change fields to be
 rationals as this is how they are typically coded.

(this structure is not referenced anywhere yet)

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libavutil/mastering_display_metadata.h | 10 ++
 libavutil/version.h|  3 +--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/libavutil/mastering_display_metadata.h b/libavutil/mastering_display_metadata.h
index 781c1fd..936533f 100644
--- a/libavutil/mastering_display_metadata.h
+++ b/libavutil/mastering_display_metadata.h
@@ -22,6 +22,8 @@
 #define AVUTIL_MASTERING_DISPLAY_METADATA_H
 
 #include "frame.h"
+#include "rational.h"
+
 
 /**
  * Mastering display metadata capable of representing the color volume of
@@ -37,22 +39,22 @@ typedef struct AVMasteringDisplayMetadata {
 /**
  * CIE 1931 xy chromaticity coords of color primaries (r, g, b order).
  */
-float display_primaries[3][2];
+AVRational display_primaries[3][2];
 
 /**
  * CIE 1931 xy chromaticity coords of white point.
  */
-float white_point[2];
+AVRational white_point[2];
 
 /**
  * Min luminance of mastering display (cd/m^2).
  */
-float min_luminance;
+AVRational min_luminance;
 
 /**
  * Max luminance of mastering display (cd/m^2).
  */
-float max_luminance;
+AVRational max_luminance;
 
 /**
  * Flag indicating whether the display primaries (and white point) are set.
diff --git a/libavutil/version.h b/libavutil/version.h
index 42f7cde..d63696d 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -65,7 +65,7 @@
 
 #define LIBAVUTIL_VERSION_MAJOR  55
 #define LIBAVUTIL_VERSION_MINOR  15
-#define LIBAVUTIL_VERSION_MICRO 100
+#define LIBAVUTIL_VERSION_MICRO 101
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
LIBAVUTIL_VERSION_MINOR, \
@@ -126,4 +126,3 @@
  */
 
 #endif /* AVUTIL_VERSION_H */
-
-- 
2.7.0.rc3.207.g0ac5344

___
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.

[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 <h.lepp...@gmail.com>
wrote:

> On Mon, Jan 25, 2016 at 10:37 PM, Michael Niedermayer
> <mich...@niedermayer.cc> 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.

[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 <neil.birkb...@gmail.com>
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 <nfx...@googlemail.com> wrote:
>
>> On Thu, 21 Jan 2016 15:57:38 -0800
>> Neil Birkbeck <neil.birkb...@gmail.com> 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

[FFmpeg-devel] [PATCH] Add missing conversions from side data enum to name

[Neil Birkbeck]

Add names for recently added enums to av_frame_side_data_name.

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libavutil/frame.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index f2097e9..92bbe09 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -728,7 +728,11 @@ const char *av_frame_side_data_name(enum 
AVFrameSideDataType type)
 case AV_FRAME_DATA_DOWNMIX_INFO:return "Metadata relevant to a downmix 
procedure";
 case AV_FRAME_DATA_REPLAYGAIN:  return "AVReplayGain";
 case AV_FRAME_DATA_DISPLAYMATRIX:   return "3x3 displaymatrix";
+case AV_FRAME_DATA_AFD: return "Active format description";
 case AV_FRAME_DATA_MOTION_VECTORS:  return "Motion vectors";
+case AV_FRAME_DATA_SKIP_SAMPLES:return "Skip samples";
+case AV_FRAME_DATA_AUDIO_SERVICE_TYPE:  return "Audio service 
type";
+case AV_FRAME_DATA_MASTERING_DISPLAY_METADATA:  return "Mastering display 
metadata";
 }
 return NULL;
 }
-- 
2.7.0.rc3.207.g0ac5344

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

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

[Neil Birkbeck]

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 <neil.birkb...@gmail.com>
---
 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;
+}
+metadata->white_point[0] = s->white_point[0] / chroma_scale;
+metadata->white_point[1] = s->white_point[1] / chroma_scale;
+metadata->max_luminance = s->max_mastering_luminance / luma_scale;
+metadata->min_luminance = s->min_mastering_luminance / luma_scale;
+metadata->has_luminance = 1;
+metadata->has_primaries = 1;
+
+av_log(s->avctx, AV_LOG_DEBUG, "Mastering Display Metadata:\n");
+av_log(s->avctx, AV_LOG_DEBUG,
+   "r(%5.4f,%5.4f) g(%5.4f,%5.4f) b(%5.4f %5.4f) wp(%5.4f, 
%5.4f)\n",
+   metadata->display_primaries[0][0], 
metadata->display_primaries[0][1],
+   metadata->display_primaries[1][0], 
metadata->display_primaries[1][1],
+   metadata->display_primaries[2][0], 
metadata->display_primaries[2][1],
+   metadata->white_point[0], metadata->white_point[1]);
+av_log(s->avctx, AV_LOG_DEBUG,
+   "min_luminance=%f, max_luminance=%f\n",
+   metadata->min_luminance, metadata->max_luminance);
+}
+
 if (s->a53_caption) {
 AVFrameSideData* sd = av_frame_new_side_data(out,
  AV_FRAME_DATA_A53_CC,
diff --git a/libavcodec/hevc.h b/libavcodec/hevc.h
index 9d72555..c64b94d 100644
--- a/libavcodec/hevc.h
+++ b/libavcodec/hevc.h
@@ -936,6 +936,13 @@ typedef struct HEVCContext {
 int sei_anticlockwise_rotation;
 int sei_hflip, sei_vflip;
 
+/** mastering display */
+int sei_mastering_display_info_present;
+int display_primaries[3][2];
+int white_point[2];
+int max_mastering_luminance;
+int min_mastering_luminance;
+
 int picture_struct;
 
 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);
+s->sei_mastering_display_info_present = 1;
+return 0;
+}
+
 static int decode_nal_sei_frame_packing_arrangement(HEVCContext *s)
 {
 GetBitContext *gb = >HEVClc->gb;
@@ -278,6 +298,8 @@ static int decode_nal_sei_prefix(HEVCContext *s, int type, 
int size)
 skip_bits(gb, 8 * size);
 return ret;
 }
+case SEI_TYPE_MASTERING_DISPLAY_INF

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

[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 <mich...@niedermayer.cc
> 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 <neil.birkb...@gmail.com>
> > ---
> >  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

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

[Neil Birkbeck]

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 <neil.birkb...@gmail.com>
---
 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;
+}
+metadata->white_point[0] = s->white_point[0] / chroma_scale;
+metadata->white_point[1] = s->white_point[1] / chroma_scale;
+metadata->max_luminance = s->max_mastering_luminance / luma_scale;
+metadata->min_luminance = s->min_mastering_luminance / luma_scale;
+metadata->has_luminance = 1;
+metadata->has_primaries = 1;
+
+av_log(s->avctx, AV_LOG_DEBUG, "Mastering Display Metadata:\n");
+av_log(s->avctx, AV_LOG_DEBUG,
+   "r(%5.4f,%5.4f) g(%5.4f,%5.4f) b(%5.4f %5.4f) wp(%5.4f, 
%5.4f)\n",
+   metadata->display_primaries[0][0], 
metadata->display_primaries[0][1],
+   metadata->display_primaries[1][0], 
metadata->display_primaries[1][1],
+   metadata->display_primaries[2][0], 
metadata->display_primaries[2][1],
+   metadata->white_point[0], metadata->white_point[1]);
+av_log(s->avctx, AV_LOG_DEBUG,
+   "min_luminance=%f, max_luminance=%f\n",
+   metadata->min_luminance, metadata->max_luminance);
+}
+
 if (s->a53_caption) {
 AVFrameSideData* sd = av_frame_new_side_data(out,
  AV_FRAME_DATA_A53_CC,
diff --git a/libavcodec/hevc.h b/libavcodec/hevc.h
index 9d72555..c91f815 100644
--- a/libavcodec/hevc.h
+++ b/libavcodec/hevc.h
@@ -941,6 +941,13 @@ typedef struct HEVCContext {
 uint8_t* a53_caption;
 int a53_caption_size;
 
+/** mastering display */
+int sei_mastering_display_info_present;
+uint16_t display_primaries[3][2];
+uint16_t white_point[2];
+uint32_t max_mastering_luminance;
+uint32_t min_mastering_luminance;
+
 } HEVCContext;
 
 int ff_hevc_decode_short_term_rps(GetBitContext *gb, AVCodecContext *avctx,
diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
index 07856f2..d33530b 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_long(gb, 32);
+s->min_mastering_luminance = get_bits_long(gb, 32);
+s->sei_mastering_display_info_present = 1;
+return 0;
+}
+
 static int decode_nal_sei_frame_packing_arrangement(HEVCContext *s)
 {
 GetBitContext *gb = >HEVClc->gb;
@@ -278,6 +298,8 @@ static int decode_nal_sei_prefix(HEVCContext *s, int type, 
int size)
 skip_bits(gb, 8 * size);
 return ret;
 }
+case S

Re: [FFmpeg-devel] [PATCH] libavutil: add mastering display metadata sidedata

[Neil Birkbeck]

Thanks for the detailed responses and discussions, and apologies for
delayed response (I was considering to change the functions to a
serialize/deserialize to some wire format and keeping the struct, but wire
format would've essentially be a memcpy to the struct on little endian and
would only benefit if there was a place that was generically writing the
SideData's data to file--wasn't sure if that was happening anywhere).

On Tue, Jan 19, 2016 at 7:39 AM, Michael Niedermayer <mich...@niedermayer.cc
> wrote:

> On Tue, Jan 19, 2016 at 01:34:12PM +0100, wm4 wrote:
> > On Tue, 19 Jan 2016 13:02:25 +0100
> > Michael Niedermayer <mich...@niedermayer.cc> wrote:
> >
> > > On Tue, Jan 19, 2016 at 10:18:16AM +0100, wm4 wrote:
> > > > On Tue, 19 Jan 2016 00:32:43 +0100
> > > > Michael Niedermayer <mich...@niedermayer.cc> wrote:
> > > >
> > > > > On Sat, Jan 16, 2016 at 04:19:38PM -0800, Neil Birkbeck wrote:
> > > > > > Adding mastering display metadata struct to avutil. The
> mastering display metadata contains information
> > > > > > about the mastering display color volume (SMPTE 2086:2014).
> > > > > >
> > > > > > This info comes from HEVC in the SEI_TYPE_MASTERING_DISPLAY_INFO
> and is soon to be included in MKV:
> > > > > >
> https://mailarchive.ietf.org/arch/search/?email_list=cellar=1=sZyfPTM-QY69P-0omfOIiTN622o
> > > > > > so it is similar to SEI FPA / stereo_mode in MKV and as such
> this patch follows how AVStereo3D is implemented.
> > > > > >
> > > > > > I'll add support to HEVC in a follow-up (and MKV when spec is
> approved).
> > > > > >
> > > > > > Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
> > > > > > ---
> > > > > >  libavutil/Makefile |  2 +
> > > > > >  libavutil/frame.h  |  7 ++-
> > > > > >  libavutil/mastering_display_metadata.c | 43 +
> > > > > >  libavutil/mastering_display_metadata.h | 87
> ++
> > > > > >  libavutil/version.h|  2 +-
> > > > > >  5 files changed, 139 insertions(+), 2 deletions(-)
> > > > > >  create mode 100644 libavutil/mastering_display_metadata.c
> > > > > >  create mode 100644 libavutil/mastering_display_metadata.h
> > > > > >
> > > > > > diff --git a/libavutil/Makefile b/libavutil/Makefile
> > > > > > index bf8c713..65b2d25 100644
> > > > > > --- a/libavutil/Makefile
> > > > > > +++ b/libavutil/Makefile
> > > > > > @@ -38,6 +38,7 @@ HEADERS = adler32.h
>  \
> > > > > >log.h
>  \
> > > > > >macros.h
> \
> > > > > >mathematics.h
>  \
> > > > > > +  mastering_display_metadata.h
> \
> > > > > >md5.h
>  \
> > > > > >mem.h
>  \
> > > > > >motion_vector.h
>  \
> > > > > > @@ -115,6 +116,7 @@ OBJS = adler32.o
> \
> > > > > > log.o
> \
> > > > > > log2_tab.o
>  \
> > > > > > mathematics.o
> \
> > > > > > +   mastering_display_metadata.o
>  \
> > > > > > md5.o
> \
> > > > > > mem.o
> \
> > > > > > murmur3.o
> \
> > > > > > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > > > > > index 9c6061a..308355b 100644
> > > > > > --- a/libavutil/frame.h
> > > > > > +++ b/libavutil/frame.h
> > > > > > @@ -106,12 +106,17 @@ enum AVFrameSideDataType {
> > > > > >   * @endcode
> > > > > >   */
> > > > > >  AV_FRAME_DATA_SKIP_SAMPLES,
> > > > > > -
> > > > > >  /**
> > > > > >   * This side data must be associated with an audio frame
> and corresponds to
> > > > > >   * enum AVAudioServiceType defined in avcodec.h.
> > > > > >   */
> > > > > >  AV_FRAME_DATA_AUDIO_SERVICE_TYPE,
> > > > >
> > > > &g

[FFmpeg-devel] [PATCH] libavutil: add mastering display metadata sidedata

[Neil Birkbeck]

Adding mastering display metadata struct to avutil. The mastering display 
metadata contains information 
about the mastering display color volume (SMPTE 2086:2014). 

This info comes from HEVC in the SEI_TYPE_MASTERING_DISPLAY_INFO and is soon to 
be included in MKV:
https://mailarchive.ietf.org/arch/search/?email_list=cellar=1=sZyfPTM-QY69P-0omfOIiTN622o
so it is similar to SEI FPA / stereo_mode in MKV and as such this patch follows 
how AVStereo3D is implemented.

I'll add support to HEVC in a follow-up (and MKV when spec is approved).

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libavutil/Makefile |  2 +
 libavutil/frame.h  |  7 ++-
 libavutil/mastering_display_metadata.c | 43 +
 libavutil/mastering_display_metadata.h | 87 ++
 libavutil/version.h|  2 +-
 5 files changed, 139 insertions(+), 2 deletions(-)
 create mode 100644 libavutil/mastering_display_metadata.c
 create mode 100644 libavutil/mastering_display_metadata.h

diff --git a/libavutil/Makefile b/libavutil/Makefile
index bf8c713..65b2d25 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -38,6 +38,7 @@ HEADERS = adler32.h   
  \
   log.h \
   macros.h  \
   mathematics.h \
+  mastering_display_metadata.h  \
   md5.h \
   mem.h \
   motion_vector.h   \
@@ -115,6 +116,7 @@ OBJS = adler32.o
\
log.o\
log2_tab.o   \
mathematics.o\
+   mastering_display_metadata.o \
md5.o\
mem.o\
murmur3.o\
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 9c6061a..308355b 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -106,12 +106,17 @@ enum AVFrameSideDataType {
  * @endcode
  */
 AV_FRAME_DATA_SKIP_SAMPLES,
-
 /**
  * This side data must be associated with an audio frame and corresponds to
  * enum AVAudioServiceType defined in avcodec.h.
  */
 AV_FRAME_DATA_AUDIO_SERVICE_TYPE,
+/**
+ * Mastering display metadata associated with a video frame. The payload is
+ * an AVMasteringDisplayMetadata type and contains information about the
+ * mastering display color volume.
+ */
+AV_FRAME_DATA_MASTERING_DISPLAY_METADATA
 };
 
 enum AVActiveFormatDescription {
diff --git a/libavutil/mastering_display_metadata.c 
b/libavutil/mastering_display_metadata.c
new file mode 100644
index 000..f7114f6
--- /dev/null
+++ b/libavutil/mastering_display_metadata.c
@@ -0,0 +1,43 @@
+/**
+ * Copyright (c) 2016 Neil Birkbeck <neil.birkb...@gmail.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include 
+#include 
+
+#include "mastering_display_metadata.h"
+#include "mem.h"
+
+AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc(void)
+{
+return av_mallocz(sizeof(AVMasteringDisplayMetadata));
+}
+
+AVMasteringDisplayMetadata 
*av_mastering_display_metadata_create_side_data(AVFrame *frame)
+{
+AVFrameSideData *side_data = av_frame_new_side_data(frame,
+
AV_FRAME_DATA_MASTERING_DISPLAY_METADATA,
+
sizeof(AVMasteringDisplayMetadata));
+if (!side_data)
+return NULL;
+
+memset(side_data->data, 0, sizeof(AVMasteringDisplayMetadata));
+
+return (AVMasteringDisplay

[FFmpeg-devel] [PATCH] libavformat/mov.c: allow QuickTime metadata to come after traks

[Neil Birkbeck]

QuickTime metadata can come after trak data. Add indicator for which trak is 
being parsed (-1 if none) so that global metadata after the trak can be parsed.

Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
---
 libavformat/isom.h | 1 +
 libavformat/mov.c  | 6 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index a082e40..65a64d4 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -178,6 +178,7 @@ typedef struct MOVContext {
 int found_moov;   ///< 'moov' atom has been found
 int found_mdat;   ///< 'mdat' atom has been found
 int found_hdlr_mdta;  ///< 'hdlr' atom with type 'mdta' has been found
+int trak_index;   ///< Index of the current 'trak'
 char **meta_keys;
 unsigned meta_keys_count;
 DVDemuxContext *dv_demux;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index cee037b..1f97d3e 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -643,7 +643,7 @@ static int mov_read_hdlr(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 av_log(c->fc, AV_LOG_TRACE, "ctype= %.4s (0x%08x)\n", (char*), 
ctype);
 av_log(c->fc, AV_LOG_TRACE, "stype= %.4s\n", (char*));
 
-if (c->fc->nb_streams < 1) {  // meta before first trak
+if (c->trak_index < 0) {  // meta not inside a trak
 if (type == MKTAG('m','d','t','a')) {
 c->found_hdlr_mdta = 1;
 }
@@ -3073,10 +3073,13 @@ static int mov_read_trak(MOVContext *c, AVIOContext 
*pb, MOVAtom atom)
 st->priv_data = sc;
 st->codec->codec_type = AVMEDIA_TYPE_DATA;
 sc->ffindex = st->index;
+c->trak_index = st->index;
 
 if ((ret = mov_read_default(c, pb, atom)) < 0)
 return ret;
 
+c->trak_index = -1;
+
 /* sanity checks */
 if (sc->chunk_count && (!sc->stts_count || !sc->stsc_count ||
 (!sc->sample_size && !sc->sample_count))) {
@@ -4617,6 +4620,7 @@ static int mov_read_header(AVFormatContext *s)
 int i;
 
 mov->fc = s;
+mov->trak_index = -1;
 /* .mov and .mp4 aren't streamable anyway (only progressive download if 
moov is before mdat) */
 if (pb->seekable)
 atom.size = avio_size(pb);
-- 
2.6.0.rc2.230.g3dd15c0

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

[FFmpeg-devel] [PATCH] libavformat/matroskaenc.c: fix small memory leaks on error

[Neil Birkbeck]

Fixing small leaks that can occur when mkv_write_tracks fails in 
mkv_write_header
(e.g., if video track has unknown codec). Also changing mkv_write_seekhead to 
take
the MatroskaMuxContext to avoid having dangling pointers.

Signed-off-by: Neil Birkbeck neil.birkb...@gmail.com
---
 libavformat/matroskaenc.c | 67 +++
 1 file changed, 44 insertions(+), 23 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 7f82804..1325c3f 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -309,6 +309,23 @@ static void put_xiph_size(AVIOContext *pb, int size)
 }
 
 /**
+ * Free the members allocated in the mux context.
+ */
+static void mkv_free(MatroskaMuxContext *mkv) {
+if (mkv-main_seekhead) {
+av_freep(mkv-main_seekhead-entries);
+av_freep(mkv-main_seekhead);
+}
+if (mkv-cues) {
+av_freep(mkv-cues-entries);
+av_freep(mkv-cues);
+}
+av_freep(mkv-tracks);
+av_freep(mkv-stream_durations);
+av_freep(mkv-stream_duration_offsets);
+}
+
+/**
  * Initialize a mkv_seekhead element to be ready to index level 1 Matroska
  * elements. If a maximum number of elements is specified, enough space
  * will be reserved at the current file location to write a seek head of
@@ -368,8 +385,9 @@ static int mkv_add_seekhead_entry(mkv_seekhead *seekhead, 
unsigned int elementid
  * @return The file offset where the seekhead was written,
  * -1 if an error occurred.
  */
-static int64_t mkv_write_seekhead(AVIOContext *pb, mkv_seekhead *seekhead)
+static int64_t mkv_write_seekhead(AVIOContext *pb, MatroskaMuxContext *mkv)
 {
+mkv_seekhead *seekhead = mkv-main_seekhead;
 ebml_master metaseek, seekentry;
 int64_t currentpos;
 int i;
@@ -406,8 +424,8 @@ static int64_t mkv_write_seekhead(AVIOContext *pb, 
mkv_seekhead *seekhead)
 currentpos = seekhead-filepos;
 }
 fail:
-av_freep(seekhead-entries);
-av_free(seekhead);
+av_freep(mkv-main_seekhead-entries);
+av_freep(mkv-main_seekhead);
 
 return currentpos;
 }
@@ -1397,9 +1415,10 @@ static int mkv_write_header(AVFormatContext *s)
 }
 
 mkv-tracks = av_mallocz_array(s-nb_streams, sizeof(*mkv-tracks));
-if (!mkv-tracks)
-return AVERROR(ENOMEM);
-
+if (!mkv-tracks) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
 ebml_header = start_ebml_master(pb, EBML_ID_HEADER, 0);
 put_ebml_uint   (pb, EBML_ID_EBMLVERSION,   1);
 put_ebml_uint   (pb, EBML_ID_EBMLREADVERSION,   1);
@@ -1419,11 +1438,13 @@ static int mkv_write_header(AVFormatContext *s)
 // isn't more than 10 elements if we only write one of each other
 // currently defined level 1 element
 mkv-main_seekhead= mkv_start_seekhead(pb, mkv-segment_offset, 10);
-if (!mkv-main_seekhead)
-return AVERROR(ENOMEM);
+if (!mkv-main_seekhead) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
 
 ret = mkv_add_seekhead_entry(mkv-main_seekhead, MATROSKA_ID_INFO, 
avio_tell(pb));
-if (ret  0) return ret;
+if (ret  0) goto fail;
 
 segment_info = start_ebml_master(pb, MATROSKA_ID_INFO, 0);
 put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 100);
@@ -1472,7 +1493,7 @@ static int mkv_write_header(AVFormatContext *s)
 
 ret = mkv_write_tracks(s);
 if (ret  0)
-return ret;
+goto fail;
 
 for (i = 0; i  s-nb_chapters; i++)
 mkv-chapter_id_offset = FFMAX(mkv-chapter_id_offset, 1LL - 
s-chapters[i]-id);
@@ -1480,24 +1501,25 @@ static int mkv_write_header(AVFormatContext *s)
 if (mkv-mode != MODE_WEBM) {
 ret = mkv_write_chapters(s);
 if (ret  0)
-return ret;
+goto fail;
 
 ret = mkv_write_tags(s);
 if (ret  0)
-return ret;
+goto fail;
 
 ret = mkv_write_attachments(s);
 if (ret  0)
-return ret;
+goto fail;
 }
 
 if (!s-pb-seekable  !mkv-is_live)
-mkv_write_seekhead(pb, mkv-main_seekhead);
+mkv_write_seekhead(pb, mkv);
 
 mkv-cues = mkv_start_cues(mkv-segment_offset);
-if (!mkv-cues)
-return AVERROR(ENOMEM);
-
+if (!mkv-cues) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
 if (pb-seekable  mkv-reserve_cues_space) {
 mkv-cues_pos = avio_tell(pb);
 put_ebml_void(pb, mkv-reserve_cues_space);
@@ -1524,6 +1546,9 @@ static int mkv_write_header(AVFormatContext *s)
 }
 
 return 0;
+fail:
+mkv_free(mkv);
+return ret;
 }
 
 static int mkv_blockgroup_size(int pkt_size)
@@ -2015,7 +2040,7 @@ static int mkv_write_trailer(AVFormatContext *s)
 return ret;
 }
 
-mkv_write_seekhead(pb, mkv-main_seekhead);
+mkv_write_seekhead(pb, mkv);
 
 // update the duration
 av_log(s, AV_LOG_DEBUG, end duration = % PRIu64 \n, mkv-duration);
@@ -2052,12 +2077,8

[FFmpeg-devel] [PATCH] avfilter/vf_yadif: fix extra leading dup frame when deint=1

[Neil Birkbeck]

Logic for handling single frame in yadif (0f9f24c9), caused deint=1 (e.g., 
yadif=0:-1:1) to output extra duplicate leading frame:

ffmpeg -i fate-suite/ffmpeg-synthetic/vsynth1/%02d.pgm  -vf 
yadif=0:-1:1,showinfo -f null -y /dev/null
 [Parsed_showinfo_1 @ 0x1d967d0] n:0 pts:0 pts_time:0 pos:-1 fmt:gray sar:0/1 
s:352x432 i:P iskey:1 type:I checksum:E457EEA0 plane_checksum:[E457EEA0] 
mean:[126] stdev:[46.6]
 [Parsed_showinfo_1 @ 0x1d967d0] n:1 pts:0 pts_time:0 pos:-1 fmt:gray sar:0/1 
s:352x432 i:P iskey:1 type:I checksum:E457EEA0 plane_checksum:[E457EEA0] 
mean:[126] stdev:[46.6]
(Outputs 51 frames)

After patch, vf yadif=0:-1:1 behaves correctly (like yadif=0:-1:0) and 
outputs 50 frames, first two:

[Parsed_showinfo_1 @ 0x1e307d0] n:0 pts:0 pts_time:0 pos:-1 fmt:gray sar:0/1 
s:352x432 i:P iskey:1 type:I checksum:68E8D1EB plane_checksum:[68E8D1EB] 
mean:[126] stdev:[46.0]
[Parsed_showinfo_1 @ 0x1e307d0] n:1 pts:2 pts_time:0.04 pos:-1 fmt:gray sar:0/1 
s:352x432 i:P iskey:1 type:I checksum:4E674BC7 plane_checksum:[4E674BC7] 
mean:[125] stdev:[46.0]
(Outputs 50 frames)

Signed-off-by: Neil Birkbeck neil.birkb...@gmail.com
---
 libavfilter/vf_yadif.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vf_yadif.c b/libavfilter/vf_yadif.c
index 70670c3..da6ee70 100644
--- a/libavfilter/vf_yadif.c
+++ b/libavfilter/vf_yadif.c
@@ -342,6 +342,9 @@ static int filter_frame(AVFilterLink *link, AVFrame *frame)
 return -1;
 }
 
+if (!yadif-prev)
+return 0;
+
 if ((yadif-deint  !yadif-cur-interlaced_frame) || ctx-is_disabled) {
 yadif-out  = av_frame_clone(yadif-cur);
 if (!yadif-out)
@@ -353,9 +356,6 @@ static int filter_frame(AVFilterLink *link, AVFrame *frame)
 return ff_filter_frame(ctx-outputs[0], yadif-out);
 }
 
-if (!yadif-prev)
-return 0;
-
 yadif-out = ff_get_video_buffer(ctx-outputs[0], link-w, link-h);
 if (!yadif-out)
 return AVERROR(ENOMEM);
-- 
2.2.0.rc0.207.ga3a616c

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

[FFmpeg-devel] [PATCH] avfilter/vf_idet: Fixing idet for single-frame inputs.

[Neil Birkbeck]

Handle single frame inputs similar to yadif (e.g., 
https://github.com/FFmpeg/FFmpeg/commit/0f9f24c9cfd291c7ece4d3bad64fdf06d107168a
 and 
https://github.com/FFmpeg/FFmpeg/commit/681e008d06d2241d50abe6316c908a184ddc5942)

Example:
  ffmpeg -r 1 -t 1 -i fate-suite/ffmpeg-synthetic/vsynth1/%02d.pgm  -vf 
idet,showinfo -f null -y /dev/null

Previously:
  Output file is empty, nothing was encoded (check -ss / -t / -frames 
parameters if used)
  [Parsed_idet_0 @ 0x36389d0] Repeated Fields: Neither: 0 Top: 0 
Bottom: 0

After patch:
  [Parsed_showinfo_1 @ 0x1909810] n:0 pts:0 pts_time:0 pos:-1 fmt:gray sar:0/1 
s:352x432 ...
  [Parsed_idet_0 @ 0x18f9bb0] Repeated Fields: Neither: 1 Top: 0 
Bottom: 0

Fate looks good.

Signed-off-by: Neil Birkbeck neil.birkb...@gmail.com

---
 libavfilter/vf_idet.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/libavfilter/vf_idet.c b/libavfilter/vf_idet.c
index ed21eea..9a25042 100644
--- a/libavfilter/vf_idet.c
+++ b/libavfilter/vf_idet.c
@@ -241,11 +241,12 @@ static int filter_frame(AVFilterLink *link, AVFrame 
*picref)
 idet-cur  = idet-next;
 idet-next = picref;
 
-if (!idet-cur)
-return 0;
+if (!idet-cur 
+!(idet-cur = av_frame_clone(idet-next)))
+return AVERROR(ENOMEM);
 
 if (!idet-prev)
-idet-prev = av_frame_clone(idet-cur);
+return 0;
 
 if (!idet-csp)
 idet-csp = av_pix_fmt_desc_get(link-format);
@@ -284,7 +285,7 @@ static int request_frame(AVFilterLink *link)
 } else if (ret  0) {
 return ret;
 }
-} while (!idet-cur);
+} while (!idet-prev);
 
 return 0;
 }
-- 
2.2.0.rc0.207.ga3a616c

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

[FFmpeg-devel] [PATCH] avfilter/vf_idet: Fixes issue with idet not flushing last frame.

[Neil Birkbeck]

Uses a similar approach as vf_yadif to flush the last frame in idet.

Quick test with 50 frames from vsynth1:
./ffmpeg.old -i fate-suite/ffmpeg-synthetic/vsynth1/%02d.pgm -vf idet -f mp4 -y 
/dev/null 21  | grep Multi
 (gives) [Parsed_idet_0 @ 0x261ebb0] Multi frame detection: TFF:0 BFF:0 
Progressive:48 Undetermined:1

./ffmpeg -i fate-suite/ffmpeg-synthetic/vsynth1/%02d.pgm -vf idet -f mp4 -y 
/dev/null 21  | grep Multi
 (gives) [Parsed_idet_0 @ 0x35a0bb0] Multi frame detection: TFF:0 BFF:0 
Progressive:49 Undetermined:1

Fate tests have been updated.

(In testing, it seems this filter will also need a subsequent patch for single 
frame input)

Signed-off-by: Neil Birkbeck neil.birkb...@gmail.com
---
 libavfilter/vf_idet.c  | 31 +++
 libavfilter/vf_idet.h  |  1 +
 tests/ref/fate/filter-idet |  2 +-
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/libavfilter/vf_idet.c b/libavfilter/vf_idet.c
index 339f4a6..b9c4070 100644
--- a/libavfilter/vf_idet.c
+++ b/libavfilter/vf_idet.c
@@ -191,6 +191,35 @@ static int filter_frame(AVFilterLink *link, AVFrame 
*picref)
 return ff_filter_frame(ctx-outputs[0], av_frame_clone(idet-cur));
 }
 
+static int request_frame(AVFilterLink *link)
+{
+AVFilterContext *ctx = link-src;
+IDETContext *idet = ctx-priv;
+
+do {
+int ret;
+
+if (idet-eof)
+return AVERROR_EOF;
+
+ret = ff_request_frame(link-src-inputs[0]);
+
+if (ret == AVERROR_EOF  idet-cur) {
+AVFrame *next = av_frame_clone(idet-next);
+
+if (!next)
+return AVERROR(ENOMEM);
+
+filter_frame(link-src-inputs[0], next);
+idet-eof = 1;
+} else if (ret  0) {
+return ret;
+}
+} while (!idet-cur);
+
+return 0;
+}
+
 static av_cold void uninit(AVFilterContext *ctx)
 {
 IDETContext *idet = ctx-priv;
@@ -253,6 +282,7 @@ static av_cold int init(AVFilterContext *ctx)
 {
 IDETContext *idet = ctx-priv;
 
+idet-eof = 0;
 idet-last_type = UNDETERMINED;
 memset(idet-history, UNDETERMINED, HIST_SIZE);
 
@@ -279,6 +309,7 @@ static const AVFilterPad idet_outputs[] = {
 .name = default,
 .type = AVMEDIA_TYPE_VIDEO,
 .config_props = config_output,
+.request_frame = request_frame
 },
 { NULL }
 };
diff --git a/libavfilter/vf_idet.h b/libavfilter/vf_idet.h
index ef29fff..57332df 100644
--- a/libavfilter/vf_idet.h
+++ b/libavfilter/vf_idet.h
@@ -50,6 +50,7 @@ typedef struct {
 ff_idet_filter_func filter_line;
 
 const AVPixFmtDescriptor *csp;
+int eof;
 } IDETContext;
 
 void ff_idet_init_x86(IDETContext *idet, int for_16b);
diff --git a/tests/ref/fate/filter-idet b/tests/ref/fate/filter-idet
index f1396c5..2f9f11c 100644
--- a/tests/ref/fate/filter-idet
+++ b/tests/ref/fate/filter-idet
@@ -1 +1 @@
-idet1790336872e844c867a53150b8ee8810
+idet005e6ddc8a5daf11cf866a1ec76c2572
-- 
2.1.0.rc2.206.gedb03e5

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