<div dir="ltr"><div>Thanks for the review <br><br></div>except uint8_t changes remaining all are white space issues, i could not use X265 Malloc, we dont have X265 Realloc, so i forced to use malloc <br></div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Wed, Sep 11, 2013 at 6:51 PM, Derek Buitenhuis <span dir="ltr"><<a href="mailto:derek.buitenhuis@gmail.com" target="_blank">derek.buitenhuis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Wed, Sep 11, 2013 at 7:47 AM, Gopu Govindaswamy<br>
<<a href="mailto:gopu@multicorewareinc.com">gopu@multicorewareinc.com</a>> wrote:<br>
> # HG changeset patch<br>
> # User Gopu Govindaswamy <<a href="mailto:gopu@multicorewareinc.com">gopu@multicorewareinc.com</a>><br>
> # Date 1378882010 -19800<br>
> # Node ID 275f2d5f9b0a782fd3ec36d95268545d96473e2e<br>
> # Parent  af8cddab103e87cb7817e4809780a64573d4dad2<br>
> NAL : std::ostringstream replaced<br>
<br>
</div>[...]<br>
<div class="im"><br>
>      /** default constructor - no initialization; must be perfomed by user */<br>
>      NALUnit() {}<br>
> -<br>
> +<br>
>      /** returns true if the NALunit is a slice NALunit */<br>
<br>
</div>Unrelated whitespace change.<br>
<div class="im"><br>
>      bool isSlice()<br>
>      {<br>
> @@ -105,7 +105,9 @@<br>
>   */<br>
>  struct NALUnitEBSP : public NALUnit<br>
>  {<br>
> -    std::ostringstream m_nalUnitData;<br>
> +<br>
<br>
</div>Unrelated whitespace change.<br>
<div class="im"><br>
> +    UInt m_packetSize;<br>
> +    unsigned char *m_nalUnitData;<br>
<br>
</div>uint8_t? Why are you using unsigned char here? This is 2013.<br>
<div class="im"><br>
> +    packetSize += bsNALUHeader.getByteStreamLength();<br>
> +    out = (unsigned char *) malloc(packetSize * sizeof(unsigned char));<br>
<br>
</div>The sizeof() here is useless.<br>
<br>
Also, why not X265_MALLOC?<br>
<div class="im"><br>
> +    memcpy(out + packetSize, &(*rbsp.begin()), rbsp.end() - rbsp.begin());<br>
<br>
</div>I cannot remember if the memory buffer behind rbsp is guaranteed to be<br>
contiguous under the C++ spec.<br>
<br>
Please ignore if this is the case, otherwise, don't do that.<br>
<div class="im"><br>
<br>
>              const NALUnitEBSP& nalu = **it;<br>
>              int size = 0; /* size of annexB unit in bytes */<br>
> -<br>
> +<br>
>              static const char start_code_prefix[] = { 0, 0, 0, 1 };<br>
<br>
</div>Unrelated whitespace change.<br>
<div class="im"><br>
><br>
>              encoder->m_nals[nalcount].i_type = nalu.m_nalUnitType;<br>
>              encoder->m_nals[nalcount].i_payload = size;<br>
>              nalcount++;<br>
> +            free(nalu.m_nalUnitData);<br>
<br>
</div>Not X264_FREE?<br>
<div class="im"><br>
>              int size = 0; /* size of annexB unit in bytes */<br>
> -<br>
> +<br>
>              static const char start_code_prefix[] = { 0, 0, 0, 1 };<br>
<br>
</div>More whitespace changes.<br>
<br>
>              nalcount++;<br>
> +            free(nalu.m_nalUnitData);<br>
>          }<br>
<br>
See above.<br>
<br>
]]<br>
<div class="im">>      else if (pi_nal)<br>
>          *pi_nal = 0;<br>
> +<br>
>      return numEncoded;<br>
<br>
</div>Ditto.<br>
<br>
- Derek<br>
_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
</blockquote></div><br><br clear="all"><br>-- <br>Thanks & Regards<br>Gopu G<br>Multicoreware Inc <br><br>
</div>