Hi James, My answers are in-line.
On Wed, Jan 29, 2020 at 8:36 AM James Zern <jzern-at-google....@ffmpeg.org> wrote: > On Fri, Jan 17, 2020 at 1:56 PM Wonkap Jang > <wonkap-at-google....@ffmpeg.org> wrote: > > > > Hi James, > > > > On Fri, Jan 17, 2020 at 1:50 PM Wonkap Jang <won...@google.com> wrote: > > > > > This commit reuses the configuration options for VP8 that enables > > > temporal scalability for VP9. It also adds a way to enable three > > > preset temporal structures (refer to the documentation for more > > > detail) that can be used in offline encoding. > > > --- > > > doc/encoders.texi | 18 ++- > > > libavcodec/libvpxenc.c | 250 +++++++++++++++++++++++++++++++++++++---- > > > 2 files changed, 242 insertions(+), 26 deletions(-) > > > > > I realized I missed a couple things previously. > > > > [...] > > > +static int vpx_ts_param_parse(VPxContext *ctx, struct > vpx_codec_enc_cfg > > > *enccfg, > > > + char *key, char *value, enum AVCodecID > > > codec_id) > > > { > > > size_t value_len = strlen(value); > > > + int ts_layering_mode = 0; > > > > > > if (!value_len) > > > return -1; > > > > > > if (!strcmp(key, "ts_number_layers")) > > There isn't much validation here, libvpx should provide that, but can you > try > some incompatible input? ts_layering_mode is defined up to 3, so > ts_number_layers > 3 should fail on init. > [WJ] actually.. it does not fail on init, since if ts_layering_mode is set to 3, then it will override that parameter to be 3. However, there are cases that are not correctly caught... such as if ts_number_layers is set to 3 but if you put 4 parameters for bitrate and such.. I do not think those cases are critical though. > > > > [...] > > > + > > > +#if (VPX_ENCODER_ABI_VERSION >= 12) && CONFIG_LIBVPX_VP9_ENCODER > > > + enccfg->temporal_layering_mode = 1; // only bypass mode is > supported > > There's VP9E_TEMPORAL_LAYERING_MODE_BYPASS for this. > [WJ] will make the change. > > > > for now. > > > + enccfg->ss_number_layers = 1; // TODO: add spatial scalability > > > support. > > > +#endif > > > + if (ts_layering_mode) { > > > + // make sure the ts_layering_mode comes at the end of the > > > ts_parameter string to ensure that > > > + // correct configuration is done. > > > + ctx->ts_layer_flags = av_malloc(sizeof(*ctx->ts_layer_flags) * > > > VPX_TS_MAX_PERIODICITY); > > This would be better written as av_malloc_array. Looking at the use of the > array, it doesn't look like it needs to be zeroed, but there's > av_mallocz_array > for that. > [WJ] will do. > _______________________________________________ > 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". Thank you, Wonkap _______________________________________________ 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".