On Thu, 2018-05-24 at 11:15 +0100, Mark Thompson wrote:
> On 24/05/18 07:29, Xiang, Haihao wrote:
> > On Wed, 2018-05-23 at 03:27 +0200, Michael Niedermayer wrote:
> > > On Mon, May 21, 2018 at 11:07:34PM +0100, Mark Thompson wrote:
> > > > On 16/05/18 08:19, Haihao Xiang wrote:
> > > > > In lavc/hevec_vaapi, colour properties in AVCodecContext are needed to
> > > > > write the sequence header
> > > > >
> > > > > Tested by the command below:
> > > > > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128
> > > > > -hwaccel_output_format vaapi -i input-with-hdr.mkv -c:v hevc_vaapi
> > > > > -profile:v main10 output.h265
> > > > >
> > > > > Signed-off-by: Haihao Xiang <haihao.xi...@intel.com>
> > > > > ---
> > > > > fftools/ffmpeg.c | 4 ++++
> > > > > 1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > > > > index 5a19a09d9a..80b5441f8f 100644
> > > > > --- a/fftools/ffmpeg.c
> > > > > +++ b/fftools/ffmpeg.c
> > > > > @@ -3298,6 +3298,10 @@ static int
> > > > > init_output_stream_encode(OutputStream
> > > > > *ost)
> > > > > dec_ctx = ist->dec_ctx;
> > > > >
> > > > > enc_ctx->chroma_sample_location = dec_ctx-
> > > > > > chroma_sample_location;
> > > > >
> > > > > + enc_ctx->color_range = dec_ctx->color_range;
> > > > > + enc_ctx->color_primaries = dec_ctx->color_primaries;
> > > > > + enc_ctx->color_trc = dec_ctx->color_trc;
> > > > > + enc_ctx->colorspace = dec_ctx->colorspace;
> > > > > } else {
> > > > > for (j = 0; j < oc->nb_streams; j++) {
> > > > > AVStream *st = oc->streams[j];
> > > > >
> > > >
> > > > There are outstanding patches passing color_range through the filter
> > > > chain
> > > > (see <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-May/229248.ht
> > > > ml>)
> > > > , and that would be the right solution for the rest of these properties
> > > > as
> > > > well, but it would require significantly more work.
> > > >
> > > > I think hacking it like this is ok for now? It's not worse than before
> > > > since a change during filtering wasn't visible anyway, and the manual
> > > > options do still work to override it. I think the commit message could
> > > > be a
> > > > little clearer about the problem (really, that this colour information
> > > > doesn't pass through filtering) and its limitations (requires a single
> > > > input
> > > > matched to the output; will write the wrong thing if filtering changes
> > > > anything) though, or maybe that could be explained in a comment.
> > > >
> > > > Does anyone else have any thoughts on this? If there are no objections
> > > > then
> > > > I would apply it updated with a clearer explanation.
> > >
> > > I think the key question is
> > > Does this change "Unknown" to incorrect values
> >
> > Do you mean the color in dec_ctx may be incorrect? If so, we should fix the
> > decoder.
>
> No, I mean where the input properties in the decode context are correct but
> don't match the output because it gets changed during filtering.
Got it, thanks
>
> For example:
>
> ffmpeg -i bt709_input.mkv -vf colorspace=bt2020 bt2020_output.mkv
>
> will have the output file marked as BT.709 after this patch, where previously
> it was "unspecified". (Explicitly setting -color_primaries/-color_trc/-
> colorspace on the output works in both cases.)
I agree with you it's not worse than before as we don't get the expected result
in both cases.
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
