Le 18/09/2013 13:47, Gopu Govindaswamy a écrit :

On Wed, Sep 18, 2013 at 5:05 PM, Jean-Baptiste Kempf <j...@videolan.org
<mailto:j...@videolan.org>> wrote:

    On 18 Sep, Gopu Govindaswamy wrote :
     > -
     > +

    whitespace issue.
     > -            AccessUnit tmp;
     > +           NALUnitEBSP **tmp = NULL;

    Identation is off
    Identation ?

Number of whitespaces

     > @@ -169,7 +169,7 @@
     >   \param   accessUnitsOut      output bitstream
     >   \retval                      number of encoded pictures
     >   */
     > -int TEncTop::encode(bool flush, const x265_picture_t* pic_in,
    x265_picture_t *pic_out, AccessUnit& accessUnitOut)
     > +int TEncTop::encode(bool flush, const x265_picture_t* pic_in,
    x265_picture_t *pic_out, NALUnitEBSP **nalunits)

    You fail to update the doxygen doc.
    I will update into doxygen doc

It should be in the same patch though...


     > -        accessUnit.insert(accessUnit.end(), new NALUnitEBSP(onalu));
     > +        //accessUnit.insert(accessUnit.end(), new
    NALUnitEBSP(onalu));

    Why?
    forgot to modify this place, after sent the patch I have sent the
    separate patch for it,

You should split those commits, if you can.

     >      UInt numRBSPBytes = 0;
     > -    for (AccessUnit::const_iterator it = accessUnit.begin(); it
    != accessUnit.end(); it++)
     > +    int count = 0;
     > +    while (nalunits[count])

    Are you sure nalunits[0] always exists?
    Yes i am sure nalunits[0] to nalunits[5] has 0 , i have initialized
    the nalunit in declaration

OK.

     >      {
     > -        UInt numRBSPBytes_nal = (*it)->m_packetSize;
     > +        UInt numRBSPBytes_nal = nalunits[count]->m_packetSize;
     >  #if VERBOSE_RATE
     >          printf("*** %6s numBytesInNALunit: %u\n",
    nalUnitTypeToString((*it)->m_nalUnitType), numRBSPBytes_nal);
     >  #endif
     > -        if ((*it)->m_nalUnitType != NAL_UNIT_PREFIX_SEI &&
    (*it)->m_nalUnitType != NAL_UNIT_SUFFIX_SEI)
     > +        if (nalunits[count]->m_nalUnitType !=
    NAL_UNIT_PREFIX_SEI && nalunits[count]->m_nalUnitType !=
    NAL_UNIT_SUFFIX_SEI)
     >          {
     >              numRBSPBytes += numRBSPBytes_nal;
     >          }
     > +        count++;
     >      }

    Why not keeping the for syntax here?
    syntax?

You transform the "for" into a "while", but you keep having a count++,

for( ; nalunits[count] != NULL; count++ )
would be a smaller change

     > +    NALUnitEBSP *nalunits[5] = { 0, 0, 0, 0, 0 };

    You should make this 5 a define, so you can use it later on

     >          if (pi_nal) *pi_nal = (int)nalcount;
     > +

    White space

     > +    NALUnitEBSP *nalunits[5] = { 0, 0, 0, 0, 0 };
     > +    int numEncoded = encoder->encode(!pic_in, pic_in, pic_out,
    nalunits);
     > +

    idem
    idem?

Idem, aka, same remark as above, aka whitespace

     > +            m_accessUnit[m_nalcount] = (NALUnitEBSP
    *)X265_MALLOC(NALUnitEBSP, 1);

    Why don't you check your malloc return???

     X265_MALLOC Will take care of this

No.

And you should really change your Mail Client to use proper quoting.

Best

--
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device
_______________________________________________
x265-devel mailing list
x265-devel@videolan.org
https://mailman.videolan.org/listinfo/x265-devel

Reply via email to