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

Steve Borho steve at borho.org
Mon Apr 6 23:48:35 CEST 2015


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)

> diff -r ebe5e57c4b45 -r e942dcdff333 source/common/param.cpp
> --- a/source/common/param.cpp	Sat Apr 04 15:11:39 2015 -0500
> +++ b/source/common/param.cpp	Mon Apr 06 01:51:57 2015 -0700
> @@ -118,6 +118,7 @@
>      param->bHighTier = 0;
>      param->interlaceMode = 0;
>      param->bRepeatHeaders = 0;
> +    param->bAnnexB = 1;
>      param->bEnableAccessUnitDelimiters = 0;
>      param->bEmitHRDSEI = 0;
>      param->bEmitInfoSEI = 1;
> diff -r ebe5e57c4b45 -r e942dcdff333 source/encoder/encoder.cpp
> --- a/source/encoder/encoder.cpp	Sat Apr 04 15:11:39 2015 -0500
> +++ b/source/encoder/encoder.cpp	Mon Apr 06 01:51:57 2015 -0700
> @@ -289,6 +289,8 @@
>      m_aborted |= parseLambdaFile(m_param);
> 
>      m_encodeStartTime = x265_mdate();
> +
> +    m_nalList.m_annexB = m_param->bAnnexB;
>  }
> 
>  void Encoder::stop()
> diff -r ebe5e57c4b45 -r e942dcdff333 source/encoder/nal.cpp
> --- a/source/encoder/nal.cpp	Sat Apr 04 15:11:39 2015 -0500
> +++ b/source/encoder/nal.cpp	Mon Apr 06 01:51:57 2015 -0700
> @@ -35,6 +35,7 @@
>      , m_extraBuffer(NULL)
>      , m_extraOccupancy(0)
>      , m_extraAllocSize(0)
> +    , m_annexB(1)
>  {}
> 
>  void NALList::takeContents(NALList& other)
> @@ -90,7 +91,12 @@
>      uint8_t *out = m_buffer + m_occupancy;
>      uint32_t bytes = 0;
> 
> -    if (!m_numNal || nalUnitType == NAL_UNIT_VPS || nalUnitType == NAL_UNIT_SPS || nalUnitType == NAL_UNIT_PPS)
> +    if (!m_annexB)
> +    {
> +        /* Will write size later */
> +        bytes += 4;
> +    }
> +    else if (!m_numNal || nalUnitType == NAL_UNIT_VPS || nalUnitType == NAL_UNIT_SPS || nalUnitType == NAL_UNIT_PPS)
>      {
>          memcpy(out, startCodePrefix, 4);
>          bytes += 4;
> @@ -144,6 +150,16 @@
>       * to 0x03 is appended to the end of the data.  */
>      if (!out[bytes - 1])
>          out[bytes++] = 0x03;
> +
> +    if (!m_annexB)
> +    {
> +        uint32_t dataSize = bytes - 4;
> +        out[0] = dataSize >> 24;
> +        out[1] = dataSize >> 16;
> +        out[2] = dataSize >>  8;
> +        out[3] = dataSize;
> +    }
> +
>      m_occupancy += bytes;
> 
>      X265_CHECK(m_numNal < (uint32_t)MAX_NAL_UNITS, "NAL count overflow\n");
> diff -r ebe5e57c4b45 -r e942dcdff333 source/encoder/nal.h
> --- a/source/encoder/nal.h	Sat Apr 04 15:11:39 2015 -0500
> +++ b/source/encoder/nal.h	Mon Apr 06 01:51:57 2015 -0700
> @@ -48,6 +48,7 @@
>      uint8_t*    m_extraBuffer;
>      uint32_t    m_extraOccupancy;
>      uint32_t    m_extraAllocSize;
> +    bool        m_annexB;
> 
>      NALList();
>      ~NALList() { X265_FREE(m_buffer); X265_FREE(m_extraBuffer); }
> diff -r ebe5e57c4b45 -r e942dcdff333 source/output/output.h
> --- a/source/output/output.h	Sat Apr 04 15:11:39 2015 -0500
> +++ b/source/output/output.h	Mon Apr 06 01:51:57 2015 -0700
> @@ -63,7 +63,7 @@
> 
>      OutputFile() {}
> 
> -    static OutputFile* open(const char *fname, InputFileInfo& inputInfo);
> +    static OutputFile* open(const char* fname, InputFileInfo& inputInfo);
> 
>      virtual bool isFail() const = 0;
> 
> @@ -71,9 +71,9 @@
> 
>      virtual void release() = 0;
> 
> -    virtual const char *getName() const = 0;
> +    virtual const char* getName() const = 0;
> 
> -    virtual void setParam(x265_param *param, x265_encoder *encoder) = 0;
> +    virtual void setParam(x265_param* param) = 0;
> 
>      virtual int writeHeaders(const x265_nal* nal, uint32_t nalcount) = 0;
> 
> diff -r ebe5e57c4b45 -r e942dcdff333 source/output/raw.cpp
> --- a/source/output/raw.cpp	Sat Apr 04 15:11:39 2015 -0500
> +++ b/source/output/raw.cpp	Mon Apr 06 01:51:57 2015 -0700
> @@ -27,7 +27,7 @@
>  using namespace x265;
>  using namespace std;
> 
> -RAWOutput::RAWOutput(const char *fname, InputFileInfo&)
> +RAWOutput::RAWOutput(const char* fname, InputFileInfo&)
>  {
>      b_fail = false;
>      if (!strcmp(fname, "-"))
> @@ -40,7 +40,10 @@
>          b_fail = true;
>  }
> 
> -void RAWOutput::setParam(x265_param *, x265_encoder *) { }
> +void RAWOutput::setParam(x265_param* param)
> +{
> +    param->bAnnexB = 1;
> +}
> 
>  int RAWOutput::writeHeaders(const x265_nal* nal, uint32_t nalcount)
>  {
> diff -r ebe5e57c4b45 -r e942dcdff333 source/output/raw.h
> --- a/source/output/raw.h	Sat Apr 04 15:11:39 2015 -0500
> +++ b/source/output/raw.h	Mon Apr 06 01:51:57 2015 -0700
> @@ -35,13 +35,13 @@
>  {
>  protected:
> 
> -    std::ostream *ofs;
> +    std::ostream* ofs;
> 
>      bool b_fail;
> 
>  public:
> 
> -    RAWOutput(const char *fname, InputFileInfo&);
> +    RAWOutput(const char* fname, InputFileInfo&);
> 
>      bool isFail() const { return b_fail; }
> 
> @@ -49,9 +49,9 @@
> 
>      void release() { delete this; }
> 
> -    const char *getName() const { return "raw"; }
> +    const char* getName() const { return "raw"; }
> 
> -    void setParam(x265_param *param, x265_encoder *);
> +    void setParam(x265_param* param);
> 
>      int writeHeaders(const x265_nal* nal, uint32_t nalcount);
> 
> diff -r ebe5e57c4b45 -r e942dcdff333 source/x265.cpp
> --- a/source/x265.cpp	Sat Apr 04 15:11:39 2015 -0500
> +++ b/source/x265.cpp	Mon Apr 06 01:51:57 2015 -0700
> @@ -483,6 +483,9 @@
>          exit(1);
>      }
> 
> +    /* This allow muxers to modify bitstream format */
> +    cliopt.output->setParam(param);
> +
>      /* note: we could try to acquire a different libx265 API here based on
>       * the profile found during option parsing, but it must be done before
>       * opening an encoder */
> @@ -527,10 +530,7 @@
>              goto fail;
>          }
>          else
> -        {
> -            cliopt.output->setParam(param, encoder);
>              cliopt.totalbytes += cliopt.output->writeHeaders(p_nal, nal);
> -        }
>      }
> 
>      api->picture_init(param, pic_in);
> diff -r ebe5e57c4b45 -r e942dcdff333 source/x265.h
> --- a/source/x265.h	Sat Apr 04 15:11:39 2015 -0500
> +++ b/source/x265.h	Mon Apr 06 01:51:57 2015 -0700
> @@ -528,6 +528,11 @@
>       * each keyframe. Default false */
>      int       bRepeatHeaders;
> 
> +    /* Flag indicating whether generating start codes (Annex B format) or size
> +     * before NAL units. Default is true, Annex B. Muxers should set this to
> +     * the correct value */
> +    int       bAnnexB;
> +
>      /* Flag indicating whether the encoder should emit an Access Unit Delimiter
>       * NAL at the start of every access unit. Default false */
>      int       bEnableAccessUnitDelimiters;
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel

-- 
Steve Borho


More information about the x265-devel mailing list