On 22/10/18 21:53, Martin Vignali wrote: > From 312b153d2b92012cacd8a189d34738de9bdfa615 Mon Sep 17 00:00:00 2001 > From: Martin Vignali <martin.vign...@gmail.com> > Date: Mon, 22 Oct 2018 22:46:05 +0200 > Subject: [PATCH 1/2] avcodec : add prores_metadata bsf for set the color > property of each prores file > > --- > doc/bitstream_filters.texi | 16 ++++++ > libavcodec/Makefile | 1 + > libavcodec/bitstream_filters.c | 1 + > libavcodec/prores_metadata_bsf.c | 120 > +++++++++++++++++++++++++++++++++++++++ > 4 files changed, 138 insertions(+) > create mode 100644 libavcodec/prores_metadata_bsf.c > > ... > diff --git a/libavcodec/prores_metadata_bsf.c > b/libavcodec/prores_metadata_bsf.c > new file mode 100644 > index 0000000000..1943cdce18 > --- /dev/null > +++ b/libavcodec/prores_metadata_bsf.c > @@ -0,0 +1,120 @@ > +/* > + * Prores Metadata bitstream filter > + * Copyright (c) 2018 Jokyo Images > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA > + */ > + > +/** > + * @file > + * Prores Metadata bitstream filter > + * set frame colorspace property > + */ > + > +#include "libavutil/common.h" > +#include "libavutil/opt.h" > + > +//#include "avcodec.h"
Dead comment. > +#include "bsf.h" > +#include "bytestream.h" > + > +typedef struct ProresMetadataContext { > + const AVClass *class; > + > + int colour_primaries; > + int transfer_characteristics; > + int matrix_coefficients; > +} ProresMetadataContext; > + > +static int prores_metadata(AVBSFContext *bsf, AVPacket *pkt) > +{ > + ProresMetadataContext *ctx = bsf->priv_data; > + int ret = 0; > + int buf_size; > + uint8_t *buf; > + > + ret = ff_bsf_get_packet_ref(bsf, pkt); > + if (ret < 0) > + return ret; > + > + buf = pkt->data; > + buf_size = pkt->size; > + > + /* check start of the prores frame */ > + if (buf_size < 28) { > + av_log(bsf, AV_LOG_ERROR, "not enough data in prores frame\n"); > + ret = AVERROR_INVALIDDATA; > + goto fail; > + } > + > + if (AV_RL32(buf + 4) != AV_RL32("icpf")) { > + av_log(bsf, AV_LOG_ERROR, "invalid frame header\n"); > + ret = AVERROR_INVALIDDATA; > + goto fail; > + } > + buf += 8; > + > + if (AV_RB16(buf) < 28) { > + av_log(bsf, AV_LOG_ERROR, "invalid frame header size\n"); > + ret = AVERROR_INVALIDDATA; > + goto fail; > + } > + > + /* set the new values */ > + buf[14] = ctx->colour_primaries; > + buf[15] = ctx->transfer_characteristics; > + buf[16] = ctx->matrix_coefficients; Since you are working in-place I think this needs a call to av_packet_make_writable() before writing. > + > +fail: > + if (ret < 0) > + av_packet_unref(pkt); > + return ret; > +} > + > ... > > From 3a5c353124cfc18ca24fda50bc5ea7c195ebcfad Mon Sep 17 00:00:00 2001 > From: Martin Vignali <martin.vign...@gmail.com> > Date: Mon, 22 Oct 2018 22:48:57 +0200 > Subject: [PATCH 2/2] fate/prores_metadata_bsf : add test for setting color > property > > --- > tests/fate/prores.mak | 18 ++++++++++++++++++ > tests/ref/fate/prores-metadata-to-rec709 | 20 ++++++++++++++++++++ > 2 files changed, 38 insertions(+) > create mode 100644 tests/ref/fate/prores-metadata-to-rec709 > > diff --git a/tests/fate/prores.mak b/tests/fate/prores.mak > index f7f52ca7fc..1a561d123a 100644 > --- a/tests/fate/prores.mak > +++ b/tests/fate/prores.mak > @@ -20,3 +20,21 @@ fate-prores-alpha_skip: CMD = framecrc -flags +bitexact > -skip_alpha 1 -i $(TARGE > fate-prores-transparency: CMD = framecrc -flags +bitexact -i > $(TARGET_SAMPLES)/prores/prores4444_with_transparency.mov -pix_fmt > yuva444p10le > fate-prores-transparency_skip: CMD = framecrc -flags +bitexact -skip_alpha 1 > -i $(TARGET_SAMPLES)/prores/prores4444_with_transparency.mov -pix_fmt > yuv444p10le > fate-prores-gray: CMD = framecrc -flags +bitexact -c:a aac_fixed -i > $(TARGET_SAMPLES)/prores/gray.mov -pix_fmt yuv422p10le > + > + > +#Test bsf prores-metadata > +tests/data/prores_proxy_rec709_metadata.mov: TAG = GEN > +tests/data/prores_proxy_rec709_metadata.mov: ffmpeg$(PROGSSUF)$(EXESUF) | > tests/data > + $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< \ > + -i $(TARGET_SAMPLES)/prores/Sequence_1-Apple_ProRes_422_Proxy.mov > -nostdin -c:v copy -an -bsf:v \ Perhaps irrelevant or intended, but this input sample already has BT.709 matrix coefficients. > + > prores_metadata=colour_primaries=1:transfer_characteristics=1:matrix_coefficients=1 > \ > + $(TARGET_PATH)/$@ -y 2>/dev/null > + > + > +FATE_PRORES_METADATA_BSF_FFPROBE += fate-prores-metadata-to-rec709 > +fate-prores-metadata-to-rec709: tests/data/prores_proxy_rec709_metadata.mov > +fate-prores-metadata-to-rec709: CMD = run ffprobe$(PROGSSUF)$(EXESUF) > -show_frames -show_entries > frame=pict_type,interlaced_frame,top_field_first,color_range,color_space,color_primaries,color_transfer,pkt_size > -v 0 $(TARGET_PATH)/tests/data/prores_proxy_rec709_metadata.mov > + > +FATE_SAMPLES_FFPROBE += $(FATE_PRORES_METADATA_BSF_FFPROBE) This is missing the dep on prores_metadata actually being enabled (try --disable-bsf=prores_metadata). I wonder whether it might be more direct to verify the output once and then just compare against the known-good hash? (That's how the CBS metadata tests work, with CMD = md5.) > + > +fate-prores-metadata-bsf: $(FATE_PRORES_METADATA_BSF_FFPROBE) > \ No newline at end of file git also has a comment for you. > ... Looks good, though +1 to the suggestion from James to write it with cbs_prores so it fits in with the other metadata BSFs (trace support for free!). Thanks, - Mark _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel