| Message ID | 15484CA6-B6B2-479C-BED0-5950A97D3E91@mac.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Mar 6, 2019 at 3:57 PM Oliver Collyer <ovcollyer@mac.com> wrote: > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > index 0ce22ec4fa..7087f82ce1 100644 > --- a/libavcodec/avcodec.h > +++ b/libavcodec/avcodec.h > @@ -3357,6 +3357,12 @@ typedef struct AVCodecContext { > * - encoding: unused > */ > int discard_damaged_percentage; > + > + /* > + * Video encoding only. Sets the maximum picture size for encoders that > + * support adjusting the picture size dynamically during encoding. > + */ > + int max_width, max_height; > } AVCodecContext; > I really don't like introducing public API fields for this. Maybe a private nvenc option would be better at this point. - Hendrik
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h >> index 0ce22ec4fa..7087f82ce1 100644 >> --- a/libavcodec/avcodec.h >> +++ b/libavcodec/avcodec.h >> @@ -3357,6 +3357,12 @@ typedef struct AVCodecContext { >> * - encoding: unused >> */ >> int discard_damaged_percentage; >> + >> + /* >> + * Video encoding only. Sets the maximum picture size for encoders that >> + * support adjusting the picture size dynamically during encoding. >> + */ >> + int max_width, max_height; >> } AVCodecContext; >> > > I really don't like introducing public API fields for this. Maybe a > private nvenc option would be better at this point. > Yeah, I just thought if you wanted to use the fields similarly for other encoders in future? Happy to change it though.
2019-03-06 15:57 GMT+01:00, Oliver Collyer <ovcollyer@mac.com>: > Hi > > I needed the dynamic resolution changing feature of NVENC to be accessible > through the ffmpeg libraries for a hobby project, so I added support and > here is a patch. > > I will format this as a proper patch after any changes necessary following > feedback. > > To use this feature you would need to: > > 1. Specify max_width and max_height before opening the encoder Can't they be set to a maximum number to be as flexible as possible? > + av_log(avctx, AV_LOG_VERBOSE, > + "resolution change: %d x %d -> %d x %d\n", > + params.reInitEncodeParams.encodeWidth, > + params.reInitEncodeParams.encodeHeight, > + (uint32_t)avctx->width, > + (uint32_t)avctx->height); These casts look strange and should be unneeded. Carl Eugen
> On 7 Mar 2019, at 23:57, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2019-03-06 15:57 GMT+01:00, Oliver Collyer <ovcollyer@mac.com>: >> Hi >> >> I needed the dynamic resolution changing feature of NVENC to be accessible >> through the ffmpeg libraries for a hobby project, so I added support and >> here is a patch. >> >> I will format this as a proper patch after any changes necessary following >> feedback. >> >> To use this feature you would need to: >> >> 1. Specify max_width and max_height before opening the encoder > > Can't they be set to a maximum number to be as flexible as possible? > It would allocate the output surfaces to this size if we took that approach and I don’t think that is particularly sensible given this feature a pretty edge use case? > >> + av_log(avctx, AV_LOG_VERBOSE, >> + "resolution change: %d x %d -> %d x %d\n", >> + params.reInitEncodeParams.encodeWidth, >> + params.reInitEncodeParams.encodeHeight, > >> + (uint32_t)avctx->width, >> + (uint32_t)avctx->height); > > These casts look strange and should be unneeded. > No idea where those came from, I blame my fingers. Will remove. > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> To use this feature you would need to: >>> >>> 1. Specify max_width and max_height before opening the encoder >> >> Can't they be set to a maximum number to be as flexible as possible? >> > > It would allocate the output surfaces to this size if we took that approach and I don’t think that is particularly sensible given this feature a pretty edge use case? > >> >>> + av_log(avctx, AV_LOG_VERBOSE, >>> + "resolution change: %d x %d -> %d x %d\n", >>> + params.reInitEncodeParams.encodeWidth, >>> + params.reInitEncodeParams.encodeHeight, >> >>> + (uint32_t)avctx->width, >>> + (uint32_t)avctx->height); >> >> These casts look strange and should be unneeded. >> > > No idea where those came from, I blame my fingers. Will remove. > V2 attached, with cast removed.
On 08/03/2019 00:57, Carl Eugen Hoyos wrote: > 2019-03-06 15:57 GMT+01:00, Oliver Collyer <ovcollyer@mac.com>: >> Hi >> >> I needed the dynamic resolution changing feature of NVENC to be accessible >> through the ffmpeg libraries for a hobby project, so I added support and >> here is a patch. >> >> I will format this as a proper patch after any changes necessary following >> feedback. >> >> To use this feature you would need to: >> >> 1. Specify max_width and max_height before opening the encoder > > Can't they be set to a maximum number to be as flexible as possible? That'd be a bad idea, as it will allocate that amount of memory for every frame, no matter how large it actually ends up being.
On 08/03/2019 07:44, Oliver Collyer wrote: > From c704e3535f866d9f89535b9df59db9ca9811bcf9 Mon Sep 17 00:00:00 2001 > From: Oliver Collyer <ovcollyer@mac.com> > Date: Fri, 8 Mar 2019 07:42:41 +0000 > Subject: [PATCH 1/1] avcodec/nvenc: Reconfigure resolution on-the-fly > > --- > libavcodec/nvenc.c | 31 ++++++++++++++++++++++++++++--- > libavcodec/nvenc.h | 3 +++ > libavcodec/nvenc_h264.c | 4 ++++ > libavcodec/nvenc_hevc.c | 4 ++++ > 4 files changed, 39 insertions(+), 3 deletions(-) Can you explain the actual intended use-case for this? The current way to handle resolution changes (or any other stream change of similar magnitude, like a pixel format change) is to flush the existing encoder and then make a new one with the new parameters. Even ignoring the trivial benefit that all encoders handle this with no additional code, it has the significant advantage that all of the stream metadata, including parameter sets, can be handled correctly. The handling here does rather badly at this - stream metadata will be misleading, and if you take one of these streams and try to put it into some container with global headers you may well end up with a quite broken file. Given that, I think there should be some justification for why you might want to do this rather than following the existing approach. Some mention in the API (avcodec.h) to explain what's going on might be a good idea, since that is currently assuming that parameters like width/height are fixed and must be set before opening the encoder. Relatedly, if there isn't any support for retrieving new metadata then it should probably display a big warning if the user tries to make a stream like this with global headers, since the global headers provided to the user on startup won't actually work for the whole stream. Thanks, - Mark
> On 12 Mar 2019, at 23:40, Mark Thompson <sw@jkqxz.net> wrote: > >> On 08/03/2019 07:44, Oliver Collyer wrote: >> From c704e3535f866d9f89535b9df59db9ca9811bcf9 Mon Sep 17 00:00:00 2001 >> From: Oliver Collyer <ovcollyer@mac.com> >> Date: Fri, 8 Mar 2019 07:42:41 +0000 >> Subject: [PATCH 1/1] avcodec/nvenc: Reconfigure resolution on-the-fly >> >> --- >> libavcodec/nvenc.c | 31 ++++++++++++++++++++++++++++--- >> libavcodec/nvenc.h | 3 +++ >> libavcodec/nvenc_h264.c | 4 ++++ >> libavcodec/nvenc_hevc.c | 4 ++++ >> 4 files changed, 39 insertions(+), 3 deletions(-) > > Can you explain the actual intended use-case for this? > > The current way to handle resolution changes (or any other stream change of similar magnitude, like a pixel format change) is to flush the existing encoder and then make a new one with the new parameters. Even ignoring the trivial benefit that all encoders handle this with no additional code, it has the significant advantage that all of the stream metadata, including parameter sets, can be handled correctly. The handling here does rather badly at this - stream metadata will be misleading, and if you take one of these streams and try to put it into some container with global headers you may well end up with a quite broken file. > I’m not sure I follow; your logic seems contradictory here - clearly if you are trying to do this in a stream with global headers you’re going to run into trouble, but during writing to such a stream whether you 1) flush, delete and re-create, or 2) reconfigure the encoder is going to have the same effect iand won’t change anything since it’s not the encoder writing any such global stream headers it’s the muxer? Or did you mean something else? I’m using it in a server app where I want to quickly and efficiently change the video size/bitrate of a transport stream sent over long distance either when the remote user requests or in response to changing network conditions, with as little disruption to the viewing experience as possible. For the record when this patch is used in conjunction with encoding into an mpegts stream it plays fine in VLC/libVLC, which defects the video changes in the stream and recreates the vout/resizes the video accordingly. > Given that, I think there should be some justification for why you might want to do this rather than following the existing approach. Some mention in the API (avcodec.h) to explain what's going on might be a good idea, since that is currently assuming that parameters like width/height are fixed and must be set before opening the encoder. Relatedly, if there isn't any support for retrieving new metadata then it should probably display a big warning if the user tries to make a stream like this with global headers, since the global headers provided to the user on startup won't actually work for the whole stream. > I think the fact this functionality is only accessible programmatically means that the bar would be quite high, ie the user likely knows what they are doing, but I can certainly put a comment next to the width/height avcodecctx members along the lines of “In some limited scenarios such as when using nvenc the width/height can be changed during encoding and will be detected by the encoder causing it to reconfigure itself to the new picture sIze. Extreme care should be taken when doing this with a format that uses global headers as the global headers will no longer correspond to the adjusted picture size!” Alternatively, maybe this is all a bit too obscure and it’s better left in my own customised ffmpeg branch? That would be quite ok, although the code does already handle dynamic bitrate and DAR changes so I figured to offer you the patch... > Thanks, > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> >> Can you explain the actual intended use-case for this? >> >> The current way to handle resolution changes (or any other stream change of similar magnitude, like a pixel format change) is to flush the existing encoder and then make a new one with the new parameters. Even ignoring the trivial benefit that all encoders handle this with no additional code, it has the significant advantage that all of the stream metadata, including parameter sets, can be handled correctly. The handling here does rather badly at this - stream metadata will be misleading, and if you take one of these streams and try to put it into some container with global headers you may well end up with a quite broken file. >> > > I’m not sure I follow; your logic seems contradictory here - clearly if you are trying to do this in a stream with global headers you’re going to run into trouble, but during writing to such a stream whether you 1) flush, delete and re-create, or 2) reconfigure the encoder is going to have the same effect iand won’t change anything since it’s not the encoder writing any such global stream headers it’s the muxer? Or did you mean something else? > > I’m using it in a server app where I want to quickly and efficiently change the video size/bitrate of a transport stream sent over long distance either when the remote user requests or in response to changing network conditions, with as little disruption to the viewing experience as possible. > > For the record when this patch is used in conjunction with encoding into an mpegts stream it plays fine in VLC/libVLC, which defects the video changes in the stream and recreates the vout/resizes the video accordingly. > >> Given that, I think there should be some justification for why you might want to do this rather than following the existing approach. Some mention in the API (avcodec.h) to explain what's going on might be a good idea, since that is currently assuming that parameters like width/height are fixed and must be set before opening the encoder. Relatedly, if there isn't any support for retrieving new metadata then it should probably display a big warning if the user tries to make a stream like this with global headers, since the global headers provided to the user on startup won't actually work for the whole stream. >> > > I think the fact this functionality is only accessible programmatically means that the bar would be quite high, ie the user likely knows what they are doing, but I can certainly put a comment next to the width/height avcodecctx members along the lines of “In some limited scenarios such as when using nvenc the width/height can be changed during encoding and will be detected by the encoder causing it to reconfigure itself to the new picture sIze. Extreme care should be taken when doing this with a format that uses global headers as the global headers will no longer correspond to the adjusted picture size!” > > Alternatively, maybe this is all a bit too obscure and it’s better left in my own customised ffmpeg branch? That would be quite ok, although the code does already handle dynamic bitrate and DAR changes so I figured to offer you the patch... > Updated patch with added comment following Mark's feedback. Regards Oliver
On 13/03/2019 01:01, Oliver Collyer wrote: >> On 12 Mar 2019, at 23:40, Mark Thompson <sw@jkqxz.net> wrote: >> >>> On 08/03/2019 07:44, Oliver Collyer wrote: >>> From c704e3535f866d9f89535b9df59db9ca9811bcf9 Mon Sep 17 00:00:00 2001 >>> From: Oliver Collyer <ovcollyer@mac.com> >>> Date: Fri, 8 Mar 2019 07:42:41 +0000 >>> Subject: [PATCH 1/1] avcodec/nvenc: Reconfigure resolution on-the-fly >>> >>> --- >>> libavcodec/nvenc.c | 31 ++++++++++++++++++++++++++++--- >>> libavcodec/nvenc.h | 3 +++ >>> libavcodec/nvenc_h264.c | 4 ++++ >>> libavcodec/nvenc_hevc.c | 4 ++++ >>> 4 files changed, 39 insertions(+), 3 deletions(-) >> >> Can you explain the actual intended use-case for this? >> >> The current way to handle resolution changes (or any other stream change of similar magnitude, like a pixel format change) is to flush the existing encoder and then make a new one with the new parameters. Even ignoring the trivial benefit that all encoders handle this with no additional code, it has the significant advantage that all of the stream metadata, including parameter sets, can be handled correctly. The handling here does rather badly at this - stream metadata will be misleading, and if you take one of these streams and try to put it into some container with global headers you may well end up with a quite broken file. >> > > I’m not sure I follow; your logic seems contradictory here - clearly if you are trying to do this in a stream with global headers you’re going to run into trouble, but during writing to such a stream whether you 1) flush, delete and re-create, or 2) reconfigure the encoder is going to have the same effect iand won’t change anything since it’s not the encoder writing any such global stream headers it’s the muxer? Or did you mean something else? Yeah, that wasn't very clear, sorry. The main point I'm trying to make on the stream metadata is that if you flush and re-create then all of that /is/ available to the user. They can of course then throw it all away to get back to the state you have if they wish, but there was at least the option to make use of it (e.g. if new global headers can be handled, maybe by starting a new fragment). I guess it's not clear to me that adding this new external interface for the special case is really a good trade-off. > I’m using it in a server app where I want to quickly and efficiently change the video size/bitrate of a transport stream sent over long distance either when the remote user requests or in response to changing network conditions, with as little disruption to the viewing experience as possible. Does the stop-start actually take longer? Hardware encoders generally try to minimise the amount of state they need to initialise and carry (because TDM), so recreating it should not be a costly operation. (Of course, I have no idea what proprietary blobs will do.) > For the record when this patch is used in conjunction with encoding into an mpegts stream it plays fine in VLC/libVLC, which defects the video changes in the stream and recreates the vout/resizes the video accordingly. Yes, I can see that it should be fine in MPEG-TS. >> Given that, I think there should be some justification for why you might want to do this rather than following the existing approach. Some mention in the API (avcodec.h) to explain what's going on might be a good idea, since that is currently assuming that parameters like width/height are fixed and must be set before opening the encoder. Relatedly, if there isn't any support for retrieving new metadata then it should probably display a big warning if the user tries to make a stream like this with global headers, since the global headers provided to the user on startup won't actually work for the whole stream. >> > > I think the fact this functionality is only accessible programmatically means that the bar would be quite high, ie the user likely knows what they are doing, but I can certainly put a comment next to the width/height avcodecctx members along the lines of “In some limited scenarios such as when using nvenc the width/height can be changed during encoding and will be detected by the encoder causing it to reconfigure itself to the new picture sIze. Extreme care should be taken when doing this with a format that uses global headers as the global headers will no longer correspond to the adjusted picture size!” Yeah, that's probably more sensible than what I suggested. > Alternatively, maybe this is all a bit too obscure and it’s better left in my own customised ffmpeg branch? That would be quite ok, although the code does already handle dynamic bitrate and DAR changes so I figured to offer you the patch... Maybe. If your use-case ends up genuinely better somehow then I think it probably would be sensible to include (and maybe investigate doing it in other cases so that nvenc isn't a surprising special-case here), but adding complexity to the library interface to save a little bit of code on the user side in one case seems a bit excessive. Thanks, - Mark
>>> Can you explain the actual intended use-case for this? >>> >>> The current way to handle resolution changes (or any other stream change of similar magnitude, like a pixel format change) is to flush the existing encoder and then make a new one with the new parameters. Even ignoring the trivial benefit that all encoders handle this with no additional code, it has the significant advantage that all of the stream metadata, including parameter sets, can be handled correctly. The handling here does rather badly at this - stream metadata will be misleading, and if you take one of these streams and try to put it into some container with global headers you may well end up with a quite broken file. >>> >> >> I’m not sure I follow; your logic seems contradictory here - clearly if you are trying to do this in a stream with global headers you’re going to run into trouble, but during writing to such a stream whether you 1) flush, delete and re-create, or 2) reconfigure the encoder is going to have the same effect iand won’t change anything since it’s not the encoder writing any such global stream headers it’s the muxer? Or did you mean something else? > > Yeah, that wasn't very clear, sorry. The main point I'm trying to make on the stream metadata is that if you flush and re-create then all of that /is/ available to the user. They can of course then throw it all away to get back to the state you have if they wish, but there was at least the option to make use of it (e.g. if new global headers can be handled, maybe by starting a new fragment). > Ok, this is probably where I'm a bit sketchy as to how things work, so please bear with me (or don't worry about this if you don't have time!!!) - so if you flush, and re-create the encoder it would write the new width/height to the output stream? In my code I roughly do this: - allocate output format context - for each input stream I am interested in, I create an output stream - find an encoder for each stream and allocate the context - set up the encoder with the parameters I need (so width, height etc) and any other options - open the encoder - copy the codec parameters to the stream (avcodec_parameters_from_context) - write header for the output format - start encoding my data I believe I originally took and adapted these steps from the ffmpeg sample code. So now, if I want to change width/height, I just change it directly in the encoder context width/height, and with this patch it picks it up and since I am only interested in mpegts it works fine. Presumably, the reconfigure does not trigger any kind of change to the output stream meta data. Whereas, you are saying that when I want to change width/height if I instead: - flush the encoder - free encoder context - create a new encoder context - set up the encoder with the new parameters - open the encoder This would give me a different result? So does opening the encoder (as opposed to reconfiguring it) fill in a bunch of output stream meta data then? Wouldn't I still need to do the step above "copy the codec parameters to the stream" and then write a new header for the output format myself? > I guess it's not clear to me that adding this new external interface for the special case is really a good trade-off. > >> I’m using it in a server app where I want to quickly and efficiently change the video size/bitrate of a transport stream sent over long distance either when the remote user requests or in response to changing network conditions, with as little disruption to the viewing experience as possible. > > Does the stop-start actually take longer? Hardware encoders generally try to minimise the amount of state they need to initialise and carry (because TDM), so recreating it should not be a costly operation. (Of course, I have no idea what proprietary blobs will do.) > That I do not know, but when I was looking at adding support for this into my project I noticed that it already handles bitrate and DAR changes so it was trivial to add resolution changes too (with the exception of needing to specify a maximum width/height). >> For the record when this patch is used in conjunction with encoding into an mpegts stream it plays fine in VLC/libVLC, which defects the video changes in the stream and recreates the vout/resizes the video accordingly. > > Yes, I can see that it should be fine in MPEG-TS. > >>> Given that, I think there should be some justification for why you might want to do this rather than following the existing approach. Some mention in the API (avcodec.h) to explain what's going on might be a good idea, since that is currently assuming that parameters like width/height are fixed and must be set before opening the encoder. Relatedly, if there isn't any support for retrieving new metadata then it should probably display a big warning if the user tries to make a stream like this with global headers, since the global headers provided to the user on startup won't actually work for the whole stream. >>> >> >> I think the fact this functionality is only accessible programmatically means that the bar would be quite high, ie the user likely knows what they are doing, but I can certainly put a comment next to the width/height avcodecctx members along the lines of “In some limited scenarios such as when using nvenc the width/height can be changed during encoding and will be detected by the encoder causing it to reconfigure itself to the new picture sIze. Extreme care should be taken when doing this with a format that uses global headers as the global headers will no longer correspond to the adjusted picture size!” > > Yeah, that's probably more sensible than what I suggested. > >> Alternatively, maybe this is all a bit too obscure and it’s better left in my own customised ffmpeg branch? That would be quite ok, although the code does already handle dynamic bitrate and DAR changes so I figured to offer you the patch... > > Maybe. If your use-case ends up genuinely better somehow then I think it probably would be sensible to include (and maybe investigate doing it in other cases so that nvenc isn't a surprising special-case here), but adding complexity to the library interface to save a little bit of code on the user side in one case seems a bit excessive. > To me it adds negligible complexity and the comment hopefully explains what to be aware of, and we already have precedent as mentioned above regarding dynamic bitrate and DAR for the nvenc encoder. However, I'm totally fine with whatever you guys decide, or happy to amend the patch further if you think it would be useful. Regards Oliver > Thanks, > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>> >>> >>> Can you explain the actual intended use-case for this? >>> >>> The current way to handle resolution changes (or any other stream change of similar magnitude, like a pixel format change) is to flush the existing encoder and then make a new one with the new parameters. Even ignoring the trivial benefit that all encoders handle this with no additional code, it has the significant advantage that all of the stream metadata, including parameter sets, can be handled correctly. The handling here does rather badly at this - stream metadata will be misleading, and if you take one of these streams and try to put it into some container with global headers you may well end up with a quite broken file. >>> >> >> I’m not sure I follow; your logic seems contradictory here - clearly if you are trying to do this in a stream with global headers you’re going to run into trouble, but during writing to such a stream whether you 1) flush, delete and re-create, or 2) reconfigure the encoder is going to have the same effect iand won’t change anything since it’s not the encoder writing any such global stream headers it’s the muxer? Or did you mean something else? >> >> I’m using it in a server app where I want to quickly and efficiently change the video size/bitrate of a transport stream sent over long distance either when the remote user requests or in response to changing network conditions, with as little disruption to the viewing experience as possible. >> >> For the record when this patch is used in conjunction with encoding into an mpegts stream it plays fine in VLC/libVLC, which defects the video changes in the stream and recreates the vout/resizes the video accordingly. >> >>> Given that, I think there should be some justification for why you might want to do this rather than following the existing approach. Some mention in the API (avcodec.h) to explain what's going on might be a good idea, since that is currently assuming that parameters like width/height are fixed and must be set before opening the encoder. Relatedly, if there isn't any support for retrieving new metadata then it should probably display a big warning if the user tries to make a stream like this with global headers, since the global headers provided to the user on startup won't actually work for the whole stream. >>> >> >> I think the fact this functionality is only accessible programmatically means that the bar would be quite high, ie the user likely knows what they are doing, but I can certainly put a comment next to the width/height avcodecctx members along the lines of “In some limited scenarios such as when using nvenc the width/height can be changed during encoding and will be detected by the encoder causing it to reconfigure itself to the new picture sIze. Extreme care should be taken when doing this with a format that uses global headers as the global headers will no longer correspond to the adjusted picture size!” >> >> Alternatively, maybe this is all a bit too obscure and it’s better left in my own customised ffmpeg branch? That would be quite ok, although the code does already handle dynamic bitrate and DAR changes so I figured to offer you the patch... >> > > Updated patch with added comment following Mark's feedback. > > Regards > > Oliver > So what's the final verdict here, can this be pushed or not? Timo - did you manage to test it over last weekend? Regards Oliver
> So what's the final verdict here, can this be pushed or not? > > Timo - did you manage to test it over last weekend? I haven't found the time, sorry. I'm generally not opposed to this. It does not disrupt normal use, and spinning up nvenc does have a surprisingly hefty overhead, so it makes sense to have features like that. One request I'd have for the patch though, if one of the new max_ parameters is set, and dyn res changing is not supported, there should be an error. Likewise if the res does change and max_width/height were not set. Silently not doing anything seems bad. Also, someone needs to ack the non-nvenc changes. Not sure if that explanation is really needed, since this is an nvenc specific edge case.
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index 0ce22ec4fa..7087f82ce1 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -3357,6 +3357,12 @@ typedef struct AVCodecContext { * - encoding: unused */ int discard_damaged_percentage; + + /* + * Video encoding only. Sets the maximum picture size for encoders that + * support adjusting the picture size dynamically during encoding. + */ + int max_width, max_height; } AVCodecContext; #if FF_API_CODEC_GET_SET diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c index 304a684e0c..414707dcfd 100644 --- a/libavcodec/nvenc.c +++ b/libavcodec/nvenc.c @@ -384,6 +384,7 @@ static int nvenc_check_capabilities(AVCodecContext *avctx) #endif ctx->support_dyn_bitrate = nvenc_check_cap(avctx, NV_ENC_CAPS_SUPPORT_DYN_BITRATE_CHANGE); + ctx->support_dyn_res = nvenc_check_cap(avctx, NV_ENC_CAPS_SUPPORT_DYN_RES_CHANGE); return 0; } @@ -1121,6 +1122,8 @@ static av_cold int nvenc_setup_encoder(AVCodecContext *avctx) ctx->init_encode_params.encodeHeight = avctx->height; ctx->init_encode_params.encodeWidth = avctx->width; + ctx->init_encode_params.maxEncodeHeight = avctx->max_height; + ctx->init_encode_params.maxEncodeWidth = avctx->max_width; ctx->init_encode_params.encodeConfig = &ctx->encode_config; @@ -1276,8 +1279,8 @@ static av_cold int nvenc_alloc_surface(AVCodecContext *avctx, int idx) } allocSurf.version = NV_ENC_CREATE_INPUT_BUFFER_VER; - allocSurf.width = avctx->width; - allocSurf.height = avctx->height; + allocSurf.width = FFMAX(avctx->max_width, avctx->width); + allocSurf.height = FFMAX(avctx->max_height, avctx->height); allocSurf.bufferFmt = ctx->surfaces[idx].format; nv_status = p_nvenc->nvEncCreateInputBuffer(ctx->nvencoder, &allocSurf); @@ -1926,7 +1929,7 @@ static void reconfig_encoder(AVCodecContext *avctx, const AVFrame *frame) NV_ENC_RECONFIGURE_PARAMS params = { 0 }; int needs_reconfig = 0; int needs_encode_config = 0; - int reconfig_bitrate = 0, reconfig_dar = 0; + int reconfig_bitrate = 0, reconfig_dar = 0, reconfig_res = 0; int dw, dh; params.version = NV_ENC_RECONFIGURE_PARAMS_VER; @@ -1986,6 +1989,24 @@ static void reconfig_encoder(AVCodecContext *avctx, const AVFrame *frame) } } + if (ctx->support_dyn_res && ctx->init_encode_params.maxEncodeWidth && ctx->init_encode_params.maxEncodeHeight) { + if (params.reInitEncodeParams.encodeWidth != avctx->width || params.reInitEncodeParams.encodeHeight != avctx->height) { + av_log(avctx, AV_LOG_VERBOSE, + "resolution change: %d x %d -> %d x %d\n", + params.reInitEncodeParams.encodeWidth, + params.reInitEncodeParams.encodeHeight, + (uint32_t)avctx->width, + (uint32_t)avctx->height); + + params.reInitEncodeParams.encodeWidth = avctx->width; + params.reInitEncodeParams.encodeHeight = avctx->height; + params.forceIDR = 1; + + needs_reconfig = 1; + reconfig_res = 1; + } + } + if (!needs_encode_config) params.reInitEncodeParams.encodeConfig = NULL; @@ -2005,6 +2026,10 @@ static void reconfig_encoder(AVCodecContext *avctx, const AVFrame *frame) ctx->encode_config.rcParams.vbvBufferSize = params.reInitEncodeParams.encodeConfig->rcParams.vbvBufferSize; } + if (reconfig_res) { + ctx->init_encode_params.encodeWidth = params.reInitEncodeParams.encodeWidth; + ctx->init_encode_params.encodeHeight = params.reInitEncodeParams.encodeHeight; + } } } } diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h index ddd6168409..f3b959b31a 100644 --- a/libavcodec/nvenc.h +++ b/libavcodec/nvenc.h @@ -158,6 +158,7 @@ typedef struct NvencContext int first_packet_output; int support_dyn_bitrate; + int support_dyn_res; void *nvencoder; diff --git a/libavcodec/version.h b/libavcodec/version.h index 309e4111cb..a217283b03 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -29,7 +29,7 @@ #define LIBAVCODEC_VERSION_MAJOR 58 #define LIBAVCODEC_VERSION_MINOR 47 -#define LIBAVCODEC_VERSION_MICRO 103 +#define LIBAVCODEC_VERSION_MICRO 104 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \