On 2/4/22 22:54, Andreas Rheinhardt wrote:
I disagree that this is the true loop condition that just needs to be
revealed; in fact, this is basically the loop condition from before
fd93d5efe64206d5f1bce8c702602353444c0c1a, just with an added block to
deal with size one units at the end. I considered this and chose the
current one because it leads to less code duplication for this special case.
Anyway, now that I have taken another look at this and I finally found
the true loop condition: Is there a unit that we have not added to the
fragment yet? This is easily implementable if one always resets the
start code, so that there being an outstanding unit is equivalent to the
start code being valid.

Looking at your patch series again, I agree your changes to cbs_mpeg2.c are clearer.

However, I think my changes from patch 11 are a further helpful clarification (ignoring the index and loop changes (since you already did that) and the "redundant" comment):

@@ -144,23 +144,24 @@  static int 
cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
                                     CodedBitstreamFragment *frag,
                                     int header)
 {
- const uint8_t *start, *end;
+ const uint8_t *start = frag->data, *end;
+ const uint8_t * const buf_end = frag->data + frag->data_size;
     CodedBitstreamUnitType unit_type;
     uint32_t start_code = -1;
     size_t unit_size;
- int err, i = 0, final = 0;
+ int err, final = 0;
+ int i = -1; // offset for pre-increment
- start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
- &start_code, 1);
+ start = avpriv_find_start_code(start, buf_end, &start_code, 1);
     if (!avpriv_start_code_is_valid(start_code)) {
         // No start code found.
         return AVERROR_INVALIDDATA;
     }
- while (!final) {
+ do {
         unit_type = start_code & 0xff;
- if (start == frag->data + frag->data_size) {
+ if (start == buf_end) {
             // The last four bytes form a start code which constitutes
             // a unit of its own.  In this situation avpriv_find_start_code
             // won't modify start_code at all so modify start_code so that
@@ -168,10 +169,9 @@  static int cbs_mpeg2_split_fragment(CodedBitstreamContext 
*ctx,
             start_code = 0;
         }
- end = avpriv_find_start_code(start--, frag->data + frag->data_size,
- &start_code, 0);
-
- // start points to the byte containing the start_code_identifier
+ end = avpriv_find_start_code(start, buf_end, &start_code, 0);
+ start--;
+ // decrement so start points to the byte containing the start_code_identifier
         // (may be the last byte of fragment->data); end points to the byte
         // following the byte containing the start code identifier (or to
         // the end of fragment->data).



Should I submit a v3 patch series which only includes patches 1-9? (That is only the avpriv_find_start_code() changes, since 10-13 were cbs_mpeg2.c and separate but related.)

Regards,
Scott

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to