[FFmpeg-devel] [RFC PATCH v6 3/4] libavcodec/j2kenc: Support for multiple layers

[Gautam Ramakrishnan]

On Thu, Aug 27, 2020 at 3:16 PM Moritz Barsnick <barsnick at gmx.net> wrote:
>
> On Wed, Aug 26, 2020 at 21:28:17 +0530, gautamramk at gmail.com wrote:
>
> > + at item layer_rates @var{string}
> > +By default, compression is done using the quality metric. This option allows for
> > +compression using compression ratio. The compression ratio for each level could
> > +be specified. The compression ratio of a layer @code{l} species the what ratio of
> > +total file size is contained in the first @code{l} layers.
>
> You should mention what the default behavior is.
That point meant that the default behavior uses quality metric rather
than compression using
layer rates. I guess it looks a bit ambiguous. I'll rephrase it.
>
> > +static int inline check_number(char* st, int* ret) {
>
> Are you reinventing strtol() with different error handling?
strol() skipped my mind. Thought of atoi(), but wanted some extra error checks.
I'll probably switch to strol()
>
> > +    token = strtok(s->lr_str, ",");
>
> There's also an av_strtok(), I wonder if that's preferred and of help.
>
> > --- a/tests/ref/vsynth/vsynth1-jpeg2000
> > +++ b/tests/ref/vsynth/vsynth1-jpeg2000
> > @@ -1,4 +1,4 @@
> > -d2a06ad916711d29b30977a06335bb76 *tests/data/fate/vsynth1-jpeg2000.avi
> > -2265698 tests/data/fate/vsynth1-jpeg2000.avi
> > -15a8e49f6fd014193bbafd72f84936c7 *tests/data/fate/vsynth1-jpeg2000.out.rawvideo
> > -stddev:    5.36 PSNR: 33.55 MAXDIFF:   61 bytes:  7603200/  7603200
> > +dd66b25f2ebc965eae4c29cfacdd960f *tests/data/fate/vsynth1-jpeg2000.avi
> > +2274950 tests/data/fate/vsynth1-jpeg2000.avi
> > +b7f48a8965f78011c76483277befc6fc *tests/data/fate/vsynth1-jpeg2000.out.rawvideo
> > +stddev:    5.35 PSNR: 33.56 MAXDIFF:   59 bytes:  7603200/  7603200
>
> [...]
>
> Is it really intended to change the default behavior when adding new
> options? I expected fate to be unchanged, unless you perhaps describe
> why a change was made.
The reason for this is that encode_packet() function is changed to
make the multi-layer encoding
possible. However, this also changes the way some values are written
for single layer encoding.
>
> In either case, whether you add options, change default behavior, or
> both, it's recommended to bump the library's micro version.
>
> Thanks,
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".



-- 
-------------
Gautam |