[FFmpeg-devel] [PATCH 1/2] avcodec/videotoolboxenc: fix undefined behavior with rc_max_rate=0

[Thomas Guillem]

On Thu, Jul 19, 2018, at 18:27, Aman Gupta wrote:
> On Fri, Jul 6, 2018 at 10:55 PM Thomas Guillem <thomas at gllm.fr> wrote:
> 
> >
> >
> > On Thu, Jul 5, 2018, at 20:49, Aman Gupta wrote:
> > > On Wed, Jul 4, 2018 at 3:05 AM Thomas Guillem <thomas at gllm.fr> wrote:
> > >
> > > > On macOS, a zero rc_max_rate cause an error from
> > > > VTSessionSetProperty(kVTCompressionPropertyKey_DataRateLimits).
> > > >
> > > > on iOS (depending on device/version), a zero rc_max_rate cause invalid
> > > > arguments from the vtenc_output_callback after few frames and then a
> > crash
> > > > within the VideoToolbox library.
> > > > ---
> > > >  libavcodec/videotoolboxenc.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/libavcodec/videotoolboxenc.c
> > b/libavcodec/videotoolboxenc.c
> > > > index ac847358ab..aa9aae7e05 100644
> > > > --- a/libavcodec/videotoolboxenc.c
> > > > +++ b/libavcodec/videotoolboxenc.c
> > > > @@ -1019,6 +1019,7 @@ static int vtenc_create_encoder(AVCodecContext
> > > >  *avctx,
> > > >
> > > >      if (vtctx->codec_id == AV_CODEC_ID_H264) {
> > > >          // kVTCompressionPropertyKey_DataRateLimits is not available
> > for
> > > > HEVC
> > > > +        if (max_rate > 0) {
> > >
> > >
> > > I think it would be better to add this condition to the existing if block
> > > above so we can avoid another level of indentation.
> >
> > That's what I first wanted to do but there is a also a profile level in
> > this code block.
> >
> 
> I split the profile block into another conditional, and added max_rate to
> this one. Applied to master and release/4.0
> 
> Aman

OK, Thanks!

> 
> 
> >
> > >
> > > Patch looks fine to me otherwise. I can make this change and commit this
> > > week.
> > >
> > > Aman
> > >
> > >
> > > >          bytes_per_second_value = max_rate >> 3;
> > > >          bytes_per_second = CFNumberCreate(kCFAllocatorDefault,
> > > >                                            kCFNumberSInt64Type,
> > > > @@ -1058,6 +1059,7 @@ static int vtenc_create_encoder(AVCodecContext
> > > >  *avctx,
> > > >              av_log(avctx, AV_LOG_ERROR, "Error setting max bitrate
> > > > property: %d\n", status);
> > > >              return AVERROR_EXTERNAL;
> > > >          }
> > > > +        }
> > > >
> > > >          if (profile_level) {
> > > >              status = VTSessionSetProperty(vtctx->session,
> > > > --
> > > > 2.18.0
> > > >
> > > > _______________________________________________
> > > > ffmpeg-devel mailing list
> > > > ffmpeg-devel at ffmpeg.org
> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel