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