On Tue, 1 Jul 2014 01:23:45 -0400
Vittorio Giovara <vittorio.giov...@gmail.com> wrote:
> Rather than using deprecated *J pixel formats, keep the full scale ones in
> a separate list, and check for avctx->color_range before opening the encoder.
>
> This slightly changes the format handshaking behaviour, instead of setting the
> first compatible format, the full range ones are always preferred (since
> they are checked last).
> ---
> This is my attempt at cleansing lavc of the yuvj pixel formats. The main
> design goal of this solution is that API is not broken, at least until the
> deprecated formats are around. This is so that our users can safely keep using
> the existing code and update it only when deprecated pixel formats are
> actually
> removed (which may be very far in time).
>
> As for the remaining libraries:
> -lavu: it should be enough to add deprecation guards;
> -lavf: unaffected;
> -lavr: unaffected;
> -lsws: until avscale is available, there are two options, either add a new
> function which solely sets pixel format color range or break
> sws_getCachedContext and add the missing color range fields there;
Yes, libswscale supports setting range perfectly fine, and the only
problem are the various insufficient APIs like sws_getCachedContext
that can't express this stuff. For what's it worth, I have an old patch
that adds an AVFrame->AVFrame conversion API to libswscale, and which
could pick up the color range (and various other things) automatically.
So the question is whether to wait on the vaporware, or whether a new
API should be added to libswscale. (To be fair, this would add only 1
new public function - or 3, if you also want to support some of the
obscure aspects of libswscale.)
> -lavfi: this is the hard one, I have a few ideas but I believe it warrants
> a separate patchset and discussion thread.
Indeed. I guess the main problem is colorspace negotiation...
>
> Comments, ideas, suggestions are always welcome.
> Vittorio
>
> doc/APIchanges | 4 ++++
> libavcodec/avcodec.h | 3 +++
> libavcodec/ljpegenc.c | 6 ++++++
> libavcodec/mjpegenc.c | 5 +++++
> libavcodec/svq3.c | 4 ++++
> libavcodec/utils.c | 29 +++++++++++++++++++++++------
> libavcodec/version.h | 5 ++++-
> 7 files changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 0e96837..be14778 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,10 @@ libavutil: 2013-12-xx
>
> API changes, most recent first:
>
> +2014-xx-xx - xxxxxxx - lavc 55.55.1 - avcodec.h
> + Add pix_fmts_full to AVCodec to list supported full range pixel formats.
> + Enabled on the next lavc major bump.
> +
> 2014-xx-xx - xxxxxxx - lsws 2.2.0
> Add sws_setColorrangeDetails() to set the input and output color range.
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index cc74aee..7ab2572 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2784,6 +2784,9 @@ typedef struct AVCodec {
> int capabilities;
> const AVRational *supported_framerates; ///< array of supported
> framerates, or NULL if any, array is terminated by {0,0}
> const enum AVPixelFormat *pix_fmts; ///< array of supported pixel
> formats, or NULL if unknown, array is terminated by -1
> +#if FF_API_PIX_FMT_FULL
> + const enum AVPixelFormat *pix_fmts_full; ///< array of supported full
> range pixel formats, or NULL if unknown, array is terminated by -1
> +#endif
> const int *supported_samplerates; ///< array of supported audio
> samplerates, or NULL if unknown, array is terminated by 0
> const enum AVSampleFormat *sample_fmts; ///< array of supported sample
> formats, or NULL if unknown, array is terminated by -1
> const uint64_t *channel_layouts; ///< array of support channel
> layouts, or NULL if unknown. array is terminated by 0
> diff --git a/libavcodec/ljpegenc.c b/libavcodec/ljpegenc.c
> index e111c8c..05b57db 100644
> --- a/libavcodec/ljpegenc.c
> +++ b/libavcodec/ljpegenc.c
> @@ -332,4 +332,10 @@ AVCodec ff_ljpeg_encoder = {
> AV_PIX_FMT_YUV422P,
> AV_PIX_FMT_YUV444P,
> AV_PIX_FMT_NONE },
> +#if FF_API_PIX_FMT_FULL
> + .pix_fmts_full = (const enum AVPixelFormat[]){ AV_PIX_FMT_YUV420P,
> + AV_PIX_FMT_YUV422P,
> + AV_PIX_FMT_YUV444P,
> + AV_PIX_FMT_NONE }
> +#endif
> };
> diff --git a/libavcodec/mjpegenc.c b/libavcodec/mjpegenc.c
> index 30433c3..6205ad7 100644
> --- a/libavcodec/mjpegenc.c
> +++ b/libavcodec/mjpegenc.c
> @@ -449,4 +449,9 @@ AVCodec ff_mjpeg_encoder = {
> .pix_fmts = (const enum AVPixelFormat[]){
> AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_NONE
> },
> +#if FF_API_PIX_FMT_FULL
> + .pix_fmts_full = (const enum AVPixelFormat[]) {
> + AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_NONE
> + },
> +#endif
> };
> diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c
> index fc2120b..e7cae2c 100644
> --- a/libavcodec/svq3.c
> +++ b/libavcodec/svq3.c
> @@ -1334,4 +1334,8 @@ AVCodec ff_svq3_decoder = {
> CODEC_CAP_DELAY,
> .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUVJ420P,
> AV_PIX_FMT_NONE},
> +#if FF_API_PIX_FMT_FULL
> + .pix_fmts_full = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUV420P,
> + AV_PIX_FMT_NONE },
> +#endif
> };
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index d6019d9..1c75f24 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -1130,7 +1130,7 @@ int attribute_align_arg avcodec_open2(AVCodecContext
> *avctx, const AVCodec *code
> avctx->thread_count = 1;
>
> if (av_codec_is_encoder(avctx->codec)) {
> - int i;
> + int i, j;
> if (avctx->codec->sample_fmts) {
> for (i = 0; avctx->codec->sample_fmts[i] != AV_SAMPLE_FMT_NONE;
> i++) {
> if (avctx->sample_fmt == avctx->codec->sample_fmts[i])
> @@ -1148,15 +1148,32 @@ int attribute_align_arg avcodec_open2(AVCodecContext
> *avctx, const AVCodec *code
> goto free_and_end;
> }
> }
> + /* Pixel formats */
> if (avctx->codec->pix_fmts) {
> for (i = 0; avctx->codec->pix_fmts[i] != AV_PIX_FMT_NONE; i++)
> +#if FF_API_PIX_FMT_FULL
> + if (avctx->pix_fmt == avctx->codec->pix_fmts[i] &&
> + avctx->color_range != AVCOL_RANGE_JPEG)
> +#else
> if (avctx->pix_fmt == avctx->codec->pix_fmts[i])
> +#endif
> break;
> - if (avctx->codec->pix_fmts[i] == AV_PIX_FMT_NONE) {
> - av_log(avctx, AV_LOG_ERROR, "Specified pix_fmt is not
> supported\n");
> - ret = AVERROR(EINVAL);
> - goto free_and_end;
> - }
> + }
> +#if FF_API_PIX_FMT_FULL
> + if (avctx->codec->pix_fmts_full) {
> + for (j = 0; avctx->codec->pix_fmts_full[j] != AV_PIX_FMT_NONE;
> j++)
> + if (avctx->pix_fmt == avctx->codec->pix_fmts_full[j] &&
> + avctx->color_range == AVCOL_RANGE_JPEG)
Assuming the color_range gets set from the (possible still J*) pixfmt
earlier, which AFAIK is already done in git master.
> + break;
> + }
> + if (avctx->codec->pix_fmts[i] == AV_PIX_FMT_NONE &&
> + avctx->codec->pix_fmts_full[j] == AV_PIX_FMT_NONE) {
> +#else
> + if (avctx->codec->pix_fmts[i] == AV_PIX_FMT_NONE) {
> +#endif
> + av_log(avctx, AV_LOG_ERROR, "Specified pix_fmt is not
> supported\n");
> + ret = AVERROR(EINVAL);
> + goto free_and_end;
> }
> if (avctx->codec->supported_samplerates) {
> for (i = 0; avctx->codec->supported_samplerates[i] != 0; i++)
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index dee6615..2f24d86 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -30,7 +30,7 @@
>
> #define LIBAVCODEC_VERSION_MAJOR 55
> #define LIBAVCODEC_VERSION_MINOR 55
> -#define LIBAVCODEC_VERSION_MICRO 0
> +#define LIBAVCODEC_VERSION_MICRO 1
>
> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> LIBAVCODEC_VERSION_MINOR, \
> @@ -144,5 +144,8 @@
> #ifndef FF_API_CODEC_NAME
> #define FF_API_CODEC_NAME (LIBAVCODEC_VERSION_MAJOR < 57)
> #endif
> +#ifndef FF_API_PIX_FMT_FULL
> +#define FF_API_PIX_FMT_FULL (LIBAVCODEC_VERSION_MAJOR >= 56)
> +#endif
>
> #endif /* AVCODEC_VERSION_H */
Like I said on IRC, IMHO this is ok for now. It's not very elegant and
doesn't cover everything you'd want to do in theory/in future, but it's
probably much better than a more complicated solution that essentially
does the same thing. It's also nicely compatible.
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
