On Thu, Oct 08, 2020 at 12:27:02PM +0100, Harry Mallon wrote:
>
>
> > On 7 Oct 2020, at 22:02, Paul B Mahol <one...@gmail.com> wrote:
> >
> > Signed-off-by: Paul B Mahol <one...@gmail.com>
> > ---
> > libavcodec/dpxenc.c | 36 ++++++++++++++++++++++++++++++++++--
> > tests/ref/lavf/dpx | 2 +-
> > 2 files changed, 35 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/dpxenc.c b/libavcodec/dpxenc.c
> > index a5960334d5..56840a8d33 100644
> > --- a/libavcodec/dpxenc.c
> > +++ b/libavcodec/dpxenc.c
> > @@ -173,6 +173,38 @@ static void encode_gbrp12(AVCodecContext *avctx, const
> > AVFrame *pic, uint16_t *d
> > }
> > }
> >
> > +static int get_dpx_pri(int color_pri)
> > +{
> > + switch (color_pri) {
> > + case AVCOL_PRI_BT709:
> > + return 6;
> > + case AVCOL_PRI_SMPTE240M:
> > + case AVCOL_PRI_SMPTE170M:
> > + return 9;
>
> I think perhaps this should be 8 (ITU 601 525), rather than 9 (Composite
> video SMPTE 170M), but I am not sure?
The smpte170m is explicitly mention in specification, so make sure you use
latest version of it.
>
> > + case AVCOL_PRI_BT470BG:
> > + return 10;
>
> Perhaps this should be 7 (ITU 601 625), rather than 10 (ITU 624-4 Composite
> video PAL), again not sure which is most widely used?
see first comment.
>
> > + default:
> > + return 0;
> > + }
> > +}
>
> In DPX files containing colour difference information the colorspace would
> also be keyed off the value returned from this function. Perhaps it should be
> taken into account here (for files containing YCbCr)?
Sorry, this does not make sense, the color_prim/trc is meaningfull for both yuv
and rgb.
>
> > +
> > +static int get_dpx_trc(int color_trc)
> > +{
> > + switch (color_trc) {
> > + case AVCOL_TRC_LINEAR:
> > + return 2;
> > + case AVCOL_TRC_BT709:
> > + return 6;
> > + case AVCOL_TRC_SMPTE240M:
> > + case AVCOL_TRC_SMPTE170M:
> > + return 9;
>
> This value could be 7, 8 or 9. From my reading of the spec the best might be
> to take colour_primaries into account and do something like:
>
> if (AVCOL_PRI_BT470BG) return 7 (ITU 601 625)
> else return 8 (ITU 601 525)
>
see first comment.
> > + case AVCOL_TRC_GAMMA22:
> > + return 10;
>
> 10 is ITU 624-4 Composite video PAL, which says it has gamma = 2.8
> (AVCOL_TRC_GAMMA28).
> https://www.itu.int/dms_pub/itu-r/opb/rep/R-REP-BT.624-4-1990-PDF-E.pdf
>
Hmm i will double check.
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> >
> > [..]
> >
>
> Best,
> 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".
