diff mbox series

[FFmpeg-devel] lavc/videotoolboxenc: set DataRateLimits for hevc

Message ID tencent_CD69B2009568667C6EADB63A89C065B7EC0A@qq.com
State Accepted
Commit 71ad83667d32f64c658034baa3cc56246ae67363
Headers show
Series [FFmpeg-devel] lavc/videotoolboxenc: set DataRateLimits for hevc | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Zhao Zhili April 25, 2021, 8:06 a.m. UTC 
From the comment it's not available on old version. It works now
by testing on macOS 11.2.1. There is no document about since when.
So trying to set the configuration and ignore the error for hevc.
---
 libavcodec/videotoolboxenc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Liu Steven April 25, 2021, 8:47 a.m. UTC | #1 
> 2021年4月25日 下午4:06，Zhao Zhili <quinkblack@foxmail.com> 写道：
> 
> From the comment it's not available on old version. It works now
> by testing on macOS 11.2.1. There is no document about since when.
> So trying to set the configuration and ignore the error for hevc.
> ---
> libavcodec/videotoolboxenc.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> index 9b7ee6720c..cefd70fa88 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -1113,8 +1113,8 @@ static int vtenc_create_encoder(AVCodecContext   *avctx,
>         return AVERROR_EXTERNAL;
>     }
> 
> -    if (vtctx->codec_id == AV_CODEC_ID_H264 && max_rate > 0) {
> -        // kVTCompressionPropertyKey_DataRateLimits is not available for HEVC
> +    if ((vtctx->codec_id == AV_CODEC_ID_H264 || vtctx->codec_id == AV_CODEC_ID_HEVC)
> +            && max_rate > 0) {
>         bytes_per_second_value = max_rate >> 3;
>         bytes_per_second = CFNumberCreate(kCFAllocatorDefault,
>                                           kCFNumberSInt64Type,
> @@ -1152,7 +1152,11 @@ static int vtenc_create_encoder(AVCodecContext   *avctx,
> 
>         if (status) {
>             av_log(avctx, AV_LOG_ERROR, "Error setting max bitrate property: %d\n", status);
> -            return AVERROR_EXTERNAL;
> +            // kVTCompressionPropertyKey_DataRateLimits is available for HEVC
> +            // now but not on old release. There is no document about since
> +            // when. So ignore the error if it failed for hevc.
> +            if (vtctx->codec_id != AV_CODEC_ID_HEVC)
> +                return AVERROR_EXTERNAL;
>         }
>     }
> 
> -- 
> 2.31.1
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 

LGTM

Thanks

Steven Liu
Rick Kern April 25, 2021, 3:31 p.m. UTC | #2 
On Sun, Apr 25, 2021 at 4:06 AM Zhao Zhili <quinkblack@foxmail.com> wrote:

> From the comment it's not available on old version. It works now
> by testing on macOS 11.2.1. There is no document about since when.
> So trying to set the configuration and ignore the error for hevc.
> ---
>  libavcodec/videotoolboxenc.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> index 9b7ee6720c..cefd70fa88 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -1113,8 +1113,8 @@ static int vtenc_create_encoder(AVCodecContext
>  *avctx,
>          return AVERROR_EXTERNAL;
>      }
>
> -    if (vtctx->codec_id == AV_CODEC_ID_H264 && max_rate > 0) {
> -        // kVTCompressionPropertyKey_DataRateLimits is not available for
> HEVC
> +    if ((vtctx->codec_id == AV_CODEC_ID_H264 || vtctx->codec_id ==
> AV_CODEC_ID_HEVC)
> +            && max_rate > 0) {
>          bytes_per_second_value = max_rate >> 3;
>          bytes_per_second = CFNumberCreate(kCFAllocatorDefault,
>                                            kCFNumberSInt64Type,
> @@ -1152,7 +1152,11 @@ static int vtenc_create_encoder(AVCodecContext
>  *avctx,
>
>          if (status) {
>              av_log(avctx, AV_LOG_ERROR, "Error setting max bitrate
> property: %d\n", status);
> -            return AVERROR_EXTERNAL;
> +            // kVTCompressionPropertyKey_DataRateLimits is available for
> HEVC
> +            // now but not on old release. There is no document about
> since
> +            // when. So ignore the error if it failed for hevc.
> +            if (vtctx->codec_id != AV_CODEC_ID_HEVC)
> +                return AVERROR_EXTERNAL;
>
The failure should be logged. Looks good otherwise.

         }
>      }
>
> --
> 2.31.1
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Zhao Zhili April 26, 2021, 7:24 a.m. UTC | #3 
> On Apr 25, 2021, at 11:31 PM, Rick Kern <kernrj@gmail.com> wrote:
> 
> On Sun, Apr 25, 2021 at 4:06 AM Zhao Zhili <quinkblack@foxmail.com> wrote:
> 
>> From the comment it's not available on old version. It works now
>> by testing on macOS 11.2.1. There is no document about since when.
>> So trying to set the configuration and ignore the error for hevc.
>> ---
>> libavcodec/videotoolboxenc.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>> 
>> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>> index 9b7ee6720c..cefd70fa88 100644
>> --- a/libavcodec/videotoolboxenc.c
>> +++ b/libavcodec/videotoolboxenc.c
>> @@ -1113,8 +1113,8 @@ static int vtenc_create_encoder(AVCodecContext
>> *avctx,
>>         return AVERROR_EXTERNAL;
>>     }
>> 
>> -    if (vtctx->codec_id == AV_CODEC_ID_H264 && max_rate > 0) {
>> -        // kVTCompressionPropertyKey_DataRateLimits is not available for
>> HEVC
>> +    if ((vtctx->codec_id == AV_CODEC_ID_H264 || vtctx->codec_id ==
>> AV_CODEC_ID_HEVC)
>> +            && max_rate > 0) {
>>         bytes_per_second_value = max_rate >> 3;
>>         bytes_per_second = CFNumberCreate(kCFAllocatorDefault,
>>                                           kCFNumberSInt64Type,
>> @@ -1152,7 +1152,11 @@ static int vtenc_create_encoder(AVCodecContext
>> *avctx,
>> 
>>         if (status) {
>>             av_log(avctx, AV_LOG_ERROR, "Error setting max bitrate
>> property: %d\n", status);
>> -            return AVERROR_EXTERNAL;
>> +            // kVTCompressionPropertyKey_DataRateLimits is available for
>> HEVC
>> +            // now but not on old release. There is no document about
>> since
>> +            // when. So ignore the error if it failed for hevc.
>> +            if (vtctx->codec_id != AV_CODEC_ID_HEVC)
>> +                return AVERROR_EXTERNAL;
>> 
> The failure should be logged. Looks good otherwise.

Yes and the failure is already logged just above the comments after `if (status)`.

> 
>         }
>>     }
>> 
>> --
>> 2.31.1
>> 
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".
>
Rick Kern May 6, 2021, 4:15 a.m. UTC | #4 
On Mon, Apr 26, 2021 at 3:24 AM "zhilizhao(赵志立)" <quinkblack@foxmail.com>
wrote:

>
>
> > On Apr 25, 2021, at 11:31 PM, Rick Kern <kernrj@gmail.com> wrote:
> >
> > On Sun, Apr 25, 2021 at 4:06 AM Zhao Zhili <quinkblack@foxmail.com>
> wrote:
> >
> >> From the comment it's not available on old version. It works now
> >> by testing on macOS 11.2.1. There is no document about since when.
> >> So trying to set the configuration and ignore the error for hevc.
> >> ---
> >> libavcodec/videotoolboxenc.c | 10 +++++++---
> >> 1 file changed, 7 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> >> index 9b7ee6720c..cefd70fa88 100644
> >> --- a/libavcodec/videotoolboxenc.c
> >> +++ b/libavcodec/videotoolboxenc.c
> >> @@ -1113,8 +1113,8 @@ static int vtenc_create_encoder(AVCodecContext
> >> *avctx,
> >>         return AVERROR_EXTERNAL;
> >>     }
> >>
> >> -    if (vtctx->codec_id == AV_CODEC_ID_H264 && max_rate > 0) {
> >> -        // kVTCompressionPropertyKey_DataRateLimits is not available
> for
> >> HEVC
> >> +    if ((vtctx->codec_id == AV_CODEC_ID_H264 || vtctx->codec_id ==
> >> AV_CODEC_ID_HEVC)
> >> +            && max_rate > 0) {
> >>         bytes_per_second_value = max_rate >> 3;
> >>         bytes_per_second = CFNumberCreate(kCFAllocatorDefault,
> >>                                           kCFNumberSInt64Type,
> >> @@ -1152,7 +1152,11 @@ static int vtenc_create_encoder(AVCodecContext
> >> *avctx,
> >>
> >>         if (status) {
> >>             av_log(avctx, AV_LOG_ERROR, "Error setting max bitrate
> >> property: %d\n", status);
> >> -            return AVERROR_EXTERNAL;
> >> +            // kVTCompressionPropertyKey_DataRateLimits is available
> for
> >> HEVC
> >> +            // now but not on old release. There is no document about
> >> since
> >> +            // when. So ignore the error if it failed for hevc.
> >> +            if (vtctx->codec_id != AV_CODEC_ID_HEVC)
> >> +                return AVERROR_EXTERNAL;
> >>
> > The failure should be logged. Looks good otherwise.
>
> Yes and the failure is already logged just above the comments after `if
> (status)`.
>
Thanks - applied.


>
> >
> >         }
> >>     }
> >>
> >> --
> >> 2.31.1
> >>
> >>
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request@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-request@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-request@ffmpeg.org with subject "unsubscribe".
>
diff mbox series

Patch

diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
index 9b7ee6720c..cefd70fa88 100644
--- a/libavcodec/videotoolboxenc.c
+++ b/libavcodec/videotoolboxenc.c
@@ -1113,8 +1113,8 @@  static int vtenc_create_encoder(AVCodecContext   *avctx,
         return AVERROR_EXTERNAL;
     }
 
-    if (vtctx->codec_id == AV_CODEC_ID_H264 && max_rate > 0) {
-        // kVTCompressionPropertyKey_DataRateLimits is not available for HEVC
+    if ((vtctx->codec_id == AV_CODEC_ID_H264 || vtctx->codec_id == AV_CODEC_ID_HEVC)
+            && max_rate > 0) {
         bytes_per_second_value = max_rate >> 3;
         bytes_per_second = CFNumberCreate(kCFAllocatorDefault,
                                           kCFNumberSInt64Type,
@@ -1152,7 +1152,11 @@  static int vtenc_create_encoder(AVCodecContext   *avctx,
 
         if (status) {
             av_log(avctx, AV_LOG_ERROR, "Error setting max bitrate property: %d\n", status);
-            return AVERROR_EXTERNAL;
+            // kVTCompressionPropertyKey_DataRateLimits is available for HEVC
+            // now but not on old release. There is no document about since
+            // when. So ignore the error if it failed for hevc.
+            if (vtctx->codec_id != AV_CODEC_ID_HEVC)
+                return AVERROR_EXTERNAL;
         }
     }