Re: [FFmpeg-devel] [PATCH v4] avformat/mxfdec: Read video range from PictureDescriptor

2020-08-20 Thread Tomas Härdin
sön 2020-08-16 klockan 11:43 +0100 skrev Harry Mallon:
> > > > http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4328/01_Bad_Frame_2.24.mxf
> > > >  
> > > > is not detected correctly for some reason.
> > > > 
> > > > The MXF specs seems ambigous:
> > > > 
> > > > Color Range is a Property, whose unsigned 32-bit integer value shall 
> > > > specify the number of distinct values allowed for color difference 
> > > > samples.
> > > > 
> > > > So probably 2^depth color range should also be accepted as full range.
> > > 
> > > This sounds correct. Do we have any sample using 2^depth-1? If not then
> > > we should just go with 2^depth until such a sample emerges.
> > > 
> > > /Tomas
> > 
> > I based it on what mxfenc.c already did, I can try to find some other 
> > samples.
> > 
> > Harry
> > 
> 
> OK, I have checked back with the docs. 
> 
> * I agree that 2^depth is correct for mxf color_range 
> * 2^depth-1 has been used in FFMPEG since n4.1 (avformat/mxfenc: add
> white/black ref /color range
> 6d0339096e10f6753049f2a5cbfd7ba69e5f8bcd) so maybe we should keep the
> off-by-one case, I don't mind either way.

Ah crap. Yeah, then we do need to accept off-by-one. We could limit
that to files that have been produced by mxfenc.c if we like. Might be
more effort than it's worth. mxfenc should be fixed either way.

> I was checking some other MXF files I have here and one is full-range 
> RGB J2K, rather than YUV. There are separate range metadata in
> RGBAEssenceDescriptor compared to CDCIEssenceDescriptor. Is there a
> way to get the stream component depth from this area of code (as RGBA
> only has component_max and component_min, no component_depth like
> CDCI) or somehow to pass the min/max to the codec to parse?
> 
> e.g in my file (RGB J2K with RGBAEssenceDesc) component_max is 4095
> and component_min is 0, but I don't think the pixel format has been
> set to 12bit yet so it would seem premature to set the range to full.

I don't know anything about J2K so I can't say

/Tomas

___
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 v4] avformat/mxfdec: Read video range from PictureDescriptor

2020-08-16 Thread Harry Mallon



> On 14 Aug 2020, at 22:33, Harry Mallon  wrote:
> 
> 
> 
> 
>> On 14 Aug 2020, at 11:53, Tomas Härdin  wrote:
>> 
>> tor 2020-08-13 klockan 22:21 +0200 skrev Marton Balint:
>>> 
>>> On Thu, 13 Aug 2020, Tomas Härdin wrote:
>>> 
 tor 2020-08-13 klockan 11:04 +0100 skrev Harry Mallon:
> Here is an updated patch (now hopefully going with correct email headers).
 
 It would be nice if in the future you either attach the patch or make
 the entire email the patch itself. I've had to trim these first couple
 of lines in each of the patches so far, after doing "git am" on the
 .mbox from saving your messages
 
> From 5866d0dc5536a6ea3f6a899c3d09f19df083c16a Mon Sep 17 00:00:00 2001
> 
> From: Harry Mallon 
> Date: Wed, 12 Aug 2020 10:26:23 +0100
> Subject: [PATCH v4] avformat/mxfdec: Read video range from 
> PictureDescriptor
> 
> * Capture black_ref, white_ref and color_range and recognise
> full and narrow range.
> 
> Signed-off-by: Harry Mallon 
> ---
> libavformat/mxfdec.c   | 29 +
> tests/ref/fate/mxf-probe-dnxhd |  2 +-
> tests/ref/fate/mxf-probe-dv25  |  2 +-
> 3 files changed, 31 insertions(+), 2 deletions(-)
 
 Looks good to me. Running FATE atm, will push in a day if there are no
 objections.
>>> 
>>> http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4328/01_Bad_Frame_2.24.mxf 
>>> is not detected correctly for some reason.
>>> 
>>> The MXF specs seems ambigous:
>>> 
>>> Color Range is a Property, whose unsigned 32-bit integer value shall 
>>> specify the number of distinct values allowed for color difference 
>>> samples.
>>> 
>>> So probably 2^depth color range should also be accepted as full range.
>> 
>> This sounds correct. Do we have any sample using 2^depth-1? If not then
>> we should just go with 2^depth until such a sample emerges.
>> 
>> /Tomas
> 
> I based it on what mxfenc.c already did, I can try to find some other samples.
> 
> Harry
> 
>> 

OK, I have checked back with the docs. 

* I agree that 2^depth is correct for mxf color_range 
* 2^depth-1 has been used in FFMPEG since n4.1 (avformat/mxfenc: add 
white/black ref /color range 6d0339096e10f6753049f2a5cbfd7ba69e5f8bcd) so maybe 
we should keep the off-by-one case, I don't mind either way.

I was checking some other MXF files I have here and one is full-range RGB J2K, 
rather than YUV. There are separate range metadata in RGBAEssenceDescriptor 
compared to CDCIEssenceDescriptor. Is there a way to get the stream component 
depth from this area of code (as RGBA only has component_max and component_min, 
no component_depth like CDCI) or somehow to pass the min/max to the codec to 
parse?

e.g in my file (RGB J2K with RGBAEssenceDesc) component_max is 4095 and 
component_min is 0, but I don't think the pixel format has been set to 12bit 
yet so it would seem premature to set the range to full.

Harry


___
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 v4] avformat/mxfdec: Read video range from PictureDescriptor

2020-08-14 Thread Harry Mallon



> On 14 Aug 2020, at 11:53, Tomas Härdin  wrote:
> 
> tor 2020-08-13 klockan 22:21 +0200 skrev Marton Balint:
>> 
>> On Thu, 13 Aug 2020, Tomas Härdin wrote:
>> 
>>> tor 2020-08-13 klockan 11:04 +0100 skrev Harry Mallon:
 Here is an updated patch (now hopefully going with correct email headers).
>>> 
>>> It would be nice if in the future you either attach the patch or make
>>> the entire email the patch itself. I've had to trim these first couple
>>> of lines in each of the patches so far, after doing "git am" on the
>>> .mbox from saving your messages
>>> 
 From 5866d0dc5536a6ea3f6a899c3d09f19df083c16a Mon Sep 17 00:00:00 2001
 
 From: Harry Mallon 
 Date: Wed, 12 Aug 2020 10:26:23 +0100
 Subject: [PATCH v4] avformat/mxfdec: Read video range from 
 PictureDescriptor
 
 * Capture black_ref, white_ref and color_range and recognise
  full and narrow range.
 
 Signed-off-by: Harry Mallon 
 ---
 libavformat/mxfdec.c   | 29 +
 tests/ref/fate/mxf-probe-dnxhd |  2 +-
 tests/ref/fate/mxf-probe-dv25  |  2 +-
 3 files changed, 31 insertions(+), 2 deletions(-)
>>> 
>>> Looks good to me. Running FATE atm, will push in a day if there are no
>>> objections.
>> 
>> http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4328/01_Bad_Frame_2.24.mxf 
>> is not detected correctly for some reason.
>> 
>> The MXF specs seems ambigous:
>> 
>> Color Range is a Property, whose unsigned 32-bit integer value shall 
>> specify the number of distinct values allowed for color difference 
>> samples.
>> 
>> So probably 2^depth color range should also be accepted as full range.
> 
> This sounds correct. Do we have any sample using 2^depth-1? If not then
> we should just go with 2^depth until such a sample emerges.
> 
> /Tomas

I based it on what mxfenc.c already did, I can try to find some other samples.

Harry

> 
> ___
> 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 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 v4] avformat/mxfdec: Read video range from PictureDescriptor

2020-08-14 Thread Tomas Härdin
tor 2020-08-13 klockan 22:21 +0200 skrev Marton Balint:
> 
> On Thu, 13 Aug 2020, Tomas Härdin wrote:
> 
> > tor 2020-08-13 klockan 11:04 +0100 skrev Harry Mallon:
> > > Here is an updated patch (now hopefully going with correct email headers).
> > 
> > It would be nice if in the future you either attach the patch or make
> > the entire email the patch itself. I've had to trim these first couple
> > of lines in each of the patches so far, after doing "git am" on the
> > .mbox from saving your messages
> > 
> > > From 5866d0dc5536a6ea3f6a899c3d09f19df083c16a Mon Sep 17 00:00:00 2001
> > > 
> > > From: Harry Mallon 
> > > Date: Wed, 12 Aug 2020 10:26:23 +0100
> > > Subject: [PATCH v4] avformat/mxfdec: Read video range from 
> > > PictureDescriptor
> > > 
> > > * Capture black_ref, white_ref and color_range and recognise
> > >   full and narrow range.
> > > 
> > > Signed-off-by: Harry Mallon 
> > > ---
> > >  libavformat/mxfdec.c   | 29 +
> > >  tests/ref/fate/mxf-probe-dnxhd |  2 +-
> > >  tests/ref/fate/mxf-probe-dv25  |  2 +-
> > >  3 files changed, 31 insertions(+), 2 deletions(-)
> > 
> > Looks good to me. Running FATE atm, will push in a day if there are no
> > objections.
> 
> http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4328/01_Bad_Frame_2.24.mxf 
> is not detected correctly for some reason.
> 
> The MXF specs seems ambigous:
> 
> Color Range is a Property, whose unsigned 32-bit integer value shall 
> specify the number of distinct values allowed for color difference 
> samples.
> 
> So probably 2^depth color range should also be accepted as full range.

This sounds correct. Do we have any sample using 2^depth-1? If not then
we should just go with 2^depth until such a sample emerges.

/Tomas

___
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 v4] avformat/mxfdec: Read video range from PictureDescriptor

2020-08-13 Thread Marton Balint



On Thu, 13 Aug 2020, Tomas Härdin wrote:


tor 2020-08-13 klockan 11:04 +0100 skrev Harry Mallon:

Here is an updated patch (now hopefully going with correct email headers).


It would be nice if in the future you either attach the patch or make
the entire email the patch itself. I've had to trim these first couple
of lines in each of the patches so far, after doing "git am" on the
.mbox from saving your messages


From 5866d0dc5536a6ea3f6a899c3d09f19df083c16a Mon Sep 17 00:00:00 2001

From: Harry Mallon 
Date: Wed, 12 Aug 2020 10:26:23 +0100
Subject: [PATCH v4] avformat/mxfdec: Read video range from PictureDescriptor

* Capture black_ref, white_ref and color_range and recognise
  full and narrow range.

Signed-off-by: Harry Mallon 
---
 libavformat/mxfdec.c   | 29 +
 tests/ref/fate/mxf-probe-dnxhd |  2 +-
 tests/ref/fate/mxf-probe-dv25  |  2 +-
 3 files changed, 31 insertions(+), 2 deletions(-)


Looks good to me. Running FATE atm, will push in a day if there are no
objections.


http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4328/01_Bad_Frame_2.24.mxf 
is not detected correctly for some reason.


The MXF specs seems ambigous:

Color Range is a Property, whose unsigned 32-bit integer value shall 
specify the number of distinct values allowed for color difference 
samples.


So probably 2^depth color range should also be accepted as full range.

Regards,
Marton
___
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 v4] avformat/mxfdec: Read video range from PictureDescriptor

2020-08-13 Thread Tomas Härdin
tor 2020-08-13 klockan 11:04 +0100 skrev Harry Mallon:
> Here is an updated patch (now hopefully going with correct email headers).

It would be nice if in the future you either attach the patch or make
the entire email the patch itself. I've had to trim these first couple
of lines in each of the patches so far, after doing "git am" on the
.mbox from saving your messages

> From 5866d0dc5536a6ea3f6a899c3d09f19df083c16a Mon Sep 17 00:00:00 2001
> 
> From: Harry Mallon 
> Date: Wed, 12 Aug 2020 10:26:23 +0100
> Subject: [PATCH v4] avformat/mxfdec: Read video range from PictureDescriptor
> 
> * Capture black_ref, white_ref and color_range and recognise
>   full and narrow range.
> 
> Signed-off-by: Harry Mallon 
> ---
>  libavformat/mxfdec.c   | 29 +
>  tests/ref/fate/mxf-probe-dnxhd |  2 +-
>  tests/ref/fate/mxf-probe-dv25  |  2 +-
>  3 files changed, 31 insertions(+), 2 deletions(-)

Looks good to me. Running FATE atm, will push in a day if there are no
objections.

/Tomas

___
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 v4] avformat/mxfdec: Read video range from PictureDescriptor

2020-08-13 Thread Harry Mallon
Here is an updated patch (now hopefully going with correct email headers).

From 5866d0dc5536a6ea3f6a899c3d09f19df083c16a Mon Sep 17 00:00:00 2001

From: Harry Mallon 
Date: Wed, 12 Aug 2020 10:26:23 +0100
Subject: [PATCH v4] avformat/mxfdec: Read video range from PictureDescriptor

* Capture black_ref, white_ref and color_range and recognise
  full and narrow range.

Signed-off-by: Harry Mallon 
---
 libavformat/mxfdec.c   | 29 +
 tests/ref/fate/mxf-probe-dnxhd |  2 +-
 tests/ref/fate/mxf-probe-dv25  |  2 +-
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 4b56984b77..0831ad5768 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -199,6 +199,9 @@ typedef struct MXFDescriptor {
 int bits_per_sample;
 int64_t duration; /* ContainerDuration optional property */
 unsigned int component_depth;
+unsigned int black_ref_level;
+unsigned int white_ref_level;
+unsigned int color_range;
 unsigned int horiz_subsampling;
 unsigned int vert_subsampling;
 UID *sub_descriptors_refs;
@@ -1222,6 +1225,15 @@ static int mxf_read_generic_descriptor(void *arg, 
AVIOContext *pb, int tag, int
 case 0x3302:
 descriptor->horiz_subsampling = avio_rb32(pb);
 break;
+case 0x3304:
+descriptor->black_ref_level = avio_rb32(pb);
+break;
+case 0x3305:
+descriptor->white_ref_level = avio_rb32(pb);
+break;
+case 0x3306:
+descriptor->color_range = avio_rb32(pb);
+break;
 case 0x3308:
 descriptor->vert_subsampling = avio_rb32(pb);
 break;
@@ -2492,6 +2504,23 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
 }
 if (descriptor->aspect_ratio.num && descriptor->aspect_ratio.den)
 st->display_aspect_ratio = descriptor->aspect_ratio;
+if (descriptor->component_depth &&
+descriptor->black_ref_level == 0 &&
+descriptor->white_ref_level == 
((1color_range == 
((1codecpar->color_range = AVCOL_RANGE_JPEG;
+}
+else if (descriptor->component_depth >= 8 &&
+ descriptor->black_ref_level == (1  
<<(descriptor->component_depth - 4)) &&
+ descriptor->white_ref_level == 
(235<<(descriptor->component_depth - 8)) &&
+ descriptor->color_range == 
((14<<(descriptor->component_depth - 4)) + 1)) {
+st->codecpar->color_range = AVCOL_RANGE_MPEG;
+}
+else if (descriptor->black_ref_level || 
descriptor->white_ref_level || descriptor->color_range) {
+avpriv_request_sample(mxf->fc, "Unrecognized color range 
(range %d, b %d, w %d, depth %d)",
+  descriptor->color_range, 
descriptor->black_ref_level,
+  descriptor->white_ref_level, 
descriptor->component_depth);
+}
 st->codecpar->color_primaries = 
mxf_get_codec_ul(ff_mxf_color_primaries_uls, 
&descriptor->color_primaries_ul)->id;
 st->codecpar->color_trc   = 
mxf_get_codec_ul(ff_mxf_color_trc_uls, &descriptor->color_trc_ul)->id;
 st->codecpar->color_space = 
mxf_get_codec_ul(ff_mxf_color_space_uls, &descriptor->color_space_ul)->id;
diff --git a/tests/ref/fate/mxf-probe-dnxhd b/tests/ref/fate/mxf-probe-dnxhd
index 012d3ea1d9..5a6221b1d5 100644
--- a/tests/ref/fate/mxf-probe-dnxhd
+++ b/tests/ref/fate/mxf-probe-dnxhd
@@ -124,7 +124,7 @@ sample_aspect_ratio=1:1
 display_aspect_ratio=4:3
 pix_fmt=yuv422p
 level=-99
-color_range=unknown
+color_range=tv
 color_space=bt709
 color_transfer=bt709
 color_primaries=bt709
diff --git a/tests/ref/fate/mxf-probe-dv25 b/tests/ref/fate/mxf-probe-dv25
index 810f145f41..ffd26c4dfd 100644
--- a/tests/ref/fate/mxf-probe-dv25
+++ b/tests/ref/fate/mxf-probe-dv25
@@ -16,7 +16,7 @@ sample_aspect_ratio=16:15
 display_aspect_ratio=4:3
 pix_fmt=yuv420p
 level=-99
-color_range=unknown
+color_range=tv
 color_space=unknown
 color_transfer=bt470m
 color_primaries=unknown
-- 
2.28.0


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