On Fri, Mar 29, 2019 at 5:28 AM Jing Sun <jing.a....@intel.com> wrote:
+static int config_enc_params(EB_H265_ENC_CONFIGURATION *param,
+ AVCodecContext *avctx)
+{
+ SvtContext *svt_enc = avctx->priv_data;
+ int ret;
+
+ param->sourceWidth = avctx->width;
+ param->sourceHeight = avctx->height;
+ param->encoderBitDepth = 8;
+
+ if (avctx->pix_fmt == AV_PIX_FMT_YUV420P10LE) {
+ av_log(avctx, AV_LOG_DEBUG, "Encoder 10 bits depth input\n");
+
+ param->encoderBitDepth = 10;
+ }
+ param->encoderColorFormat = EB_YUV420;
>this patch blatantly ignores my review(s) and therefore it is rejected
>to reiterate:
>- if the encoder does not support 10 bit (or if scales down from 10 to 8
>internally), this feature should not be present in the wrapper either (or it
>should at least warn the user)
>- if the encoder does not support setting the color properties in the VUI, the
>wrapper should definitely warn the user of the loss of information (or we
>should wait until this features is present upstream)
>reagrds
>--
>Vittorio
----------------------------------
So what is happening in this case? The encoder is slower because it converts
from 10 to 8 bit internally? And then the output encode is 8 bit right now? If
that is the case, I'd rather have this functionality removed since the
conversion can happen directly within ffmpeg. Having the conversion performed
by the encoder is guaranteed to be slower and less precise, and if the output
is not 10 bit it is very surprising too.
At the same time the comment in the code is useless because users will never
read something buried deep in the code, I'd suggest printing something at the
warning level so that it will be shown during the conversion (and please have
it proofread by a native English-speaking person).
---------------------------------
You said that functionality should be removed and the comment in the code is
useless, so I removed it. The 10 bit works fine this way, no need to warn the
users.
----------------------------------
nit: excessive whitespace alignment
----------------------------------
I have corrected the whitespace.
----------------------------------
Where is the warning that notifies the lack of color properties support?
----------------------------------
I was with you, so added the warning in v9, then I removed the hdr/vui
interfaces entirely in v10, after I saw mark's review. I think you are right,
that the SVT-HEVC's implementation of hdr/vui is not a complete one , hence the
removal. And I have explained the removal in the former reply: to add hdr back
once the feature is fully supported in SVT-HEVC.
----------------------------------
IMO these options help text could be improved.
----------------------------------
I have improved it in v9/v10.
+ { "aud", "Include Access Unit Delimiter", OFFSET(aud),
+ AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
- Jing
_______________________________________________
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".
