[x265] [PATCH for review] cli: annex_b format switch

[Steve Borho]

On 04/07, Xinyue Lu wrote:
> Checklist
> =========
> 
> 1. x265_param_parse
> If I understand correctly, this is supposed to support user-input command line
>  options, which I don't think is necessary here. AnnexB switch ties to muxers,
>  which are inside CLI. Even user changes the switch, it will be overwritten by
>  muxers anyway. For those who uses C API, they should set the correct flag in
>  x265_param according to the muxers (eg ffmpeg should set the flag upon output
>  format, not from encoding options).

x265_param_parse() is used by the CLI, *and* by users of the C API. It
is preferred to set params by name than by setting x265_param fields
directly, since x265_param_parse() is generally kept backwards
compatible when API changes are made. x265_param_parse() is more
future-proof

> 2. x265_param2string
> And this is to write options into SEI. I don't think bitstream format needs to
>  be written into SEI because it will not match the format if user remuxes (or
>  demuxes) the stream into other format.

I checked x264 common.c, they support annexb in param_parse but not in
param2string(). I'm fine following their example. The annexB flag would
have to be known just to parse the stream to read the SEI

> 3. getopt
> Said above.
> 
> 4. cli.rst
> Said above.

all params supported by x265_param_parse() need to be documented in the
reST.

> 5. X265_BUILD
> Will increase from 54 to 55 in CMakeList.txt.

ok

> 6. tests
> Due to not changing the CLI, I don't think we need to change the command line
>  of tests.

ok

> Comments, suggestions, please.
> 
> ??? 2015/4/6 14:48, Steve Borho ??????:
> > On 04/06, Xinyue Lu wrote:
> >> # HG changeset patch
> >> # User Xinyue Lu <i at 7086.in>
> >> # Date 1428310317 25200
> >> #      Mon Apr 06 01:51:57 2015 -0700
> >> # Branch Yuuki
> >> # Node ID e942dcdff333e5c8b30d1e524314ab1aa5fd481e
> >> # Parent  ebe5e57c4b45b45338035a1009b64585f21d66d5
> >> cli: annex_b format switch
> >>
> >> When bAnnexB set to true, the nal serializer will generate start codes (0x00 00 00 01) before payload.
> >> When false, it will generate size before payload.
> >> Container formats may prefer the latter.
> >>
> >> Also move output->setParam up so that it can select format before we initialize the encoder.
> > 
> > The general logic is good, but when you add a new field to x265_param
> > there are a lot of responsibilities to take care of:
> > 
> > 1. x265_param_parse() must be updated
> > 2. x265_param2string() must be updated
> > 3. getopt() support needs to be added to x265cli.h (and CLI help)
> > 4. the param must be documented in doc/reST/cli.rst
> > 5. X265_BUILD must be incremented since we are changing the binary
> >    interface
> > 6. tests have to be added to smoke-tests.txt and regression-tests.txt.
> >    in this case, at least one smoke test and one regression test should
> >    use --no-annexb (with raw output)
> 
> By the way, raw output needs annexb format, while mkv / mp4 containers love
>  the file format (no-annexb).
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel

-- 
Steve Borho