diff mbox series

[FFmpeg-devel,v5,2/2] libavformat/mxfenc: color_range should be inclusive

Message ID 20200820135853.96869-2-harry.mallon@codex.online
State Accepted
Commit abd58a4192e4c5ea721b22365c211d8fa874f3d2
Headers show
Series [FFmpeg-devel,v5,1/2] avformat/mxfdec: Read video range from CDCIEssenceDescriptor | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Harry Mallon Aug. 20, 2020, 1:58 p.m. UTC
MXF CDCI color range was being set to (1<<sc->component_depth) - 1
for full range but it should be (1<<sc->component_depth) is 0 is
a valid value.

Signed-off-by: Harry Mallon <harry.mallon@codex.online>
---
 libavformat/mxfenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tomas Härdin Aug. 24, 2020, 8:30 a.m. UTC | #1
tor 2020-08-20 klockan 14:58 +0100 skrev Harry Mallon:
> MXF CDCI color range was being set to (1<<sc->component_depth) - 1
> for full range but it should be (1<<sc->component_depth) is 0 is
> a valid value.

Grammar here is a bit strange. Do you mean 0 is a valid value? Looks OK
besides this

/Tomas
Harry Mallon Aug. 24, 2020, 11:02 a.m. UTC | #2
> On 24 Aug 2020, at 09:30, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> 
> tor 2020-08-20 klockan 14:58 +0100 skrev Harry Mallon:
>> MXF CDCI color range was being set to (1<<sc->component_depth) - 1
>> for full range but it should be (1<<sc->component_depth) is 0 is
>> a valid value.
> 
> Grammar here is a bit strange. Do you mean 0 is a valid value? Looks OK
> besides this

Sorry, it is meant to read ‘as 0 is a valid value’. Shall I change in a new version?

Thanks for being attentive and speedy with my patches btw. Sending things into FFMPEG has been pretty painless. :)

Harry.
Tomas Härdin Aug. 29, 2020, 11:52 a.m. UTC | #3
mån 2020-08-24 klockan 12:02 +0100 skrev Harry Mallon:
> > On 24 Aug 2020, at 09:30, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> > 
> > tor 2020-08-20 klockan 14:58 +0100 skrev Harry Mallon:
> > > MXF CDCI color range was being set to (1<<sc->component_depth) -
> > > 1
> > > for full range but it should be (1<<sc->component_depth) is 0 is
> > > a valid value.
> > 
> > Grammar here is a bit strange. Do you mean 0 is a valid value?
> > Looks OK
> > besides this
> 
> Sorry, it is meant to read ‘as 0 is a valid value’. Shall I change in
> a new version?
> 
> Thanks for being attentive and speedy with my patches btw. Sending
> things into FFMPEG has been pretty painless. :)

Both patches pushed. This was being held up by fate-cfhd-1 being broken

/Tomas
diff mbox series

Patch

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index a38fa6b983..e495b5ba0e 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -1160,7 +1160,7 @@  static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
     if (st->codecpar->color_range != AVCOL_RANGE_UNSPECIFIED) {
         int black = 0,
             white = (1<<sc->component_depth) - 1,
-            color = (1<<sc->component_depth) - 1;
+            color = (1<<sc->component_depth);
         if (st->codecpar->color_range == AVCOL_RANGE_MPEG) {
             black = 1   << (sc->component_depth - 4);
             white = 235 << (sc->component_depth - 8);