| Message ID | 20180522185713.20122-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | df3d962d2e736c0748b91a3c8c701d7cb96154a8 |
| Headers | show |
On 5/22/2018 3:57 PM, Michael Niedermayer wrote: > May fix ticket7209 > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/mxfenc.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c > index 44425bf6c9..77f60f5874 100644 > --- a/libavformat/mxfenc.c > +++ b/libavformat/mxfenc.c > @@ -1149,23 +1149,23 @@ static int get_trc(UID ul, enum AVColorTransferCharacteristic trc) > switch (trc){ > case AVCOL_TRC_GAMMA28 : > case AVCOL_TRC_GAMMA22 : > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00}, 16); > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00}), 16); > return 0; > case AVCOL_TRC_BT709 : > case AVCOL_TRC_SMPTE170M : > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00}, 16); > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00}), 16); > return 0; > case AVCOL_TRC_SMPTE240M : > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x03,0x00,0x00}, 16); > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x03,0x00,0x00}), 16); > return 0; > case AVCOL_TRC_BT1361_ECG: > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x05,0x00,0x00}, 16); > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x05,0x00,0x00}), 16); > return 0; > case AVCOL_TRC_LINEAR : > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x06,0x00,0x00}, 16); > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x06,0x00,0x00}), 16); > return 0; > case AVCOL_TRC_SMPTE428 : > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x08,0x04,0x01,0x01,0x01,0x01,0x07,0x00,0x00}, 16); > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x08,0x04,0x01,0x01,0x01,0x01,0x07,0x00,0x00}), 16); > return 0; > default: > return -1; Why not just declare these as static const like mxf_mpeg2_codec_uls or the descriptor keys right above this function?
On Tue, May 22, 2018 at 06:13:47PM -0300, James Almer wrote: > On 5/22/2018 3:57 PM, Michael Niedermayer wrote: > > May fix ticket7209 > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/mxfenc.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c > > index 44425bf6c9..77f60f5874 100644 > > --- a/libavformat/mxfenc.c > > +++ b/libavformat/mxfenc.c > > @@ -1149,23 +1149,23 @@ static int get_trc(UID ul, enum AVColorTransferCharacteristic trc) > > switch (trc){ > > case AVCOL_TRC_GAMMA28 : > > case AVCOL_TRC_GAMMA22 : > > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00}, 16); > > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00}), 16); > > return 0; > > case AVCOL_TRC_BT709 : > > case AVCOL_TRC_SMPTE170M : > > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00}, 16); > > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00}), 16); > > return 0; > > case AVCOL_TRC_SMPTE240M : > > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x03,0x00,0x00}, 16); > > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x03,0x00,0x00}), 16); > > return 0; > > case AVCOL_TRC_BT1361_ECG: > > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x05,0x00,0x00}, 16); > > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x05,0x00,0x00}), 16); > > return 0; > > case AVCOL_TRC_LINEAR : > > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x06,0x00,0x00}, 16); > > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x06,0x00,0x00}), 16); > > return 0; > > case AVCOL_TRC_SMPTE428 : > > - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x08,0x04,0x01,0x01,0x01,0x01,0x07,0x00,0x00}, 16); > > + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x08,0x04,0x01,0x01,0x01,0x01,0x07,0x00,0x00}), 16); > > return 0; > > default: > > return -1; > > Why not just declare these as static const like mxf_mpeg2_codec_uls or > the descriptor keys right above this function? possible but it would seperate the values from the switch/case, making it harder to see what is stored by what. If people prefer i can change it to that ? I think its more readable as is / with the patch above though [...]
On 5/23/2018 9:38 PM, Michael Niedermayer wrote: > On Tue, May 22, 2018 at 06:13:47PM -0300, James Almer wrote: >> On 5/22/2018 3:57 PM, Michael Niedermayer wrote: >>> May fix ticket7209 >>> >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavformat/mxfenc.c | 12 ++++++------ >>> 1 file changed, 6 insertions(+), 6 deletions(-) >>> >>> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c >>> index 44425bf6c9..77f60f5874 100644 >>> --- a/libavformat/mxfenc.c >>> +++ b/libavformat/mxfenc.c >>> @@ -1149,23 +1149,23 @@ static int get_trc(UID ul, enum AVColorTransferCharacteristic trc) >>> switch (trc){ >>> case AVCOL_TRC_GAMMA28 : >>> case AVCOL_TRC_GAMMA22 : >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00}, 16); >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00}), 16); >>> return 0; >>> case AVCOL_TRC_BT709 : >>> case AVCOL_TRC_SMPTE170M : >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00}, 16); >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00}), 16); >>> return 0; >>> case AVCOL_TRC_SMPTE240M : >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x03,0x00,0x00}, 16); >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x03,0x00,0x00}), 16); >>> return 0; >>> case AVCOL_TRC_BT1361_ECG: >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x05,0x00,0x00}, 16); >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x05,0x00,0x00}), 16); >>> return 0; >>> case AVCOL_TRC_LINEAR : >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x06,0x00,0x00}, 16); >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x06,0x00,0x00}), 16); >>> return 0; >>> case AVCOL_TRC_SMPTE428 : >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x08,0x04,0x01,0x01,0x01,0x01,0x07,0x00,0x00}, 16); >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x08,0x04,0x01,0x01,0x01,0x01,0x07,0x00,0x00}), 16); >>> return 0; >>> default: >>> return -1; >> >> Why not just declare these as static const like mxf_mpeg2_codec_uls or >> the descriptor keys right above this function? > > possible but it would seperate the values from the switch/case, making > it harder to see what is stored by what. > If people prefer i can change it to that ? > I think its more readable as is / with the patch above though It's fine as is with me. I think it looks odd when compared to other functions writing UID constants, so it was a suggestion based on consistency more than anything.
On Thu, May 24, 2018 at 01:39:00PM -0300, James Almer wrote: > On 5/23/2018 9:38 PM, Michael Niedermayer wrote: > > On Tue, May 22, 2018 at 06:13:47PM -0300, James Almer wrote: > >> On 5/22/2018 3:57 PM, Michael Niedermayer wrote: > >>> May fix ticket7209 > >>> > >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > >>> --- > >>> libavformat/mxfenc.c | 12 ++++++------ > >>> 1 file changed, 6 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c > >>> index 44425bf6c9..77f60f5874 100644 > >>> --- a/libavformat/mxfenc.c > >>> +++ b/libavformat/mxfenc.c > >>> @@ -1149,23 +1149,23 @@ static int get_trc(UID ul, enum AVColorTransferCharacteristic trc) > >>> switch (trc){ > >>> case AVCOL_TRC_GAMMA28 : > >>> case AVCOL_TRC_GAMMA22 : > >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00}, 16); > >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00}), 16); > >>> return 0; > >>> case AVCOL_TRC_BT709 : > >>> case AVCOL_TRC_SMPTE170M : > >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00}, 16); > >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00}), 16); > >>> return 0; > >>> case AVCOL_TRC_SMPTE240M : > >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x03,0x00,0x00}, 16); > >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x03,0x00,0x00}), 16); > >>> return 0; > >>> case AVCOL_TRC_BT1361_ECG: > >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x05,0x00,0x00}, 16); > >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x05,0x00,0x00}), 16); > >>> return 0; > >>> case AVCOL_TRC_LINEAR : > >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x06,0x00,0x00}, 16); > >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x06,0x00,0x00}), 16); > >>> return 0; > >>> case AVCOL_TRC_SMPTE428 : > >>> - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x08,0x04,0x01,0x01,0x01,0x01,0x07,0x00,0x00}, 16); > >>> + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x08,0x04,0x01,0x01,0x01,0x01,0x07,0x00,0x00}), 16); > >>> return 0; > >>> default: > >>> return -1; > >> > >> Why not just declare these as static const like mxf_mpeg2_codec_uls or > >> the descriptor keys right above this function? > > > > possible but it would seperate the values from the switch/case, making > > it harder to see what is stored by what. > > If people prefer i can change it to that ? > > I think its more readable as is / with the patch above though > > It's fine as is with me. I think it looks odd when compared to other > functions writing UID constants, so it was a suggestion based on > consistency more than anything. will apply thanks [...]
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index 44425bf6c9..77f60f5874 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -1149,23 +1149,23 @@ static int get_trc(UID ul, enum AVColorTransferCharacteristic trc) switch (trc){ case AVCOL_TRC_GAMMA28 : case AVCOL_TRC_GAMMA22 : - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00}, 16); + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00}), 16); return 0; case AVCOL_TRC_BT709 : case AVCOL_TRC_SMPTE170M : - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00}, 16); + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00}), 16); return 0; case AVCOL_TRC_SMPTE240M : - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x03,0x00,0x00}, 16); + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x03,0x00,0x00}), 16); return 0; case AVCOL_TRC_BT1361_ECG: - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x05,0x00,0x00}, 16); + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x05,0x00,0x00}), 16); return 0; case AVCOL_TRC_LINEAR : - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x06,0x00,0x00}, 16); + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x06,0x00,0x00}), 16); return 0; case AVCOL_TRC_SMPTE428 : - memcpy(ul, (UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x08,0x04,0x01,0x01,0x01,0x01,0x07,0x00,0x00}, 16); + memcpy(ul, ((UID){0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x08,0x04,0x01,0x01,0x01,0x01,0x07,0x00,0x00}), 16); return 0; default: return -1;
May fix ticket7209 Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/mxfenc.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)