| Message ID | 1568078324-8655-1-git-send-email-lance.lmwang@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 9/9/2019 10:18 PM, lance.lmwang@gmail.com wrote: > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > libavcodec/h2645_parse.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c > index 307e8643e6..403acd2ee1 100644 > --- a/libavcodec/h2645_parse.c > +++ b/libavcodec/h2645_parse.c > @@ -454,7 +454,7 @@ int ff_h2645_packet_split(H2645Packet *pkt, const uint8_t *buf, int length, > } > > if (pkt->nals_allocated < pkt->nb_nals + 1) { > - int new_size = pkt->nals_allocated + 1; > + int new_size = pkt->nb_nals + 1; No. nals_allocated and nb_nals are two purposely separate values in order to reuse the H2645Packet struct. One keeps track of the allocated size of the NAL buffer, whereas the other keeps track of the amount of NALs used by the current packet. That's why the check immediately above this line makes sure the amount of NALs in the current packet fit in the current size of the NAL buffer, and grows/reallocates it otherwise. > void *tmp = av_realloc_array(pkt->nals, new_size, sizeof(*pkt->nals)); > > if (!tmp) >
On Mon, Sep 09, 2019 at 10:53:53PM -0300, James Almer wrote: > On 9/9/2019 10:18 PM, lance.lmwang@gmail.com wrote: > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > libavcodec/h2645_parse.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c > > index 307e8643e6..403acd2ee1 100644 > > --- a/libavcodec/h2645_parse.c > > +++ b/libavcodec/h2645_parse.c > > @@ -454,7 +454,7 @@ int ff_h2645_packet_split(H2645Packet *pkt, const uint8_t *buf, int length, > > } > > > > if (pkt->nals_allocated < pkt->nb_nals + 1) { > > - int new_size = pkt->nals_allocated + 1; > > + int new_size = pkt->nb_nals + 1; > > No. nals_allocated and nb_nals are two purposely separate values in > order to reuse the H2645Packet struct. One keeps track of the allocated > size of the NAL buffer, whereas the other keeps track of the amount of > NALs used by the current packet. That's why the check immediately above > this line makes sure the amount of NALs in the current packet fit in the > current size of the NAL buffer, and grows/reallocates it otherwise. Thanks for the detail information about the two field. Below is my assumation as I haven't such sample to test such case. Assuming nals_allocated is 1, nb_nals is 3, then the current code will reallocate new_size with 2, But the following code will use the nb_nals to access pkt->nals, I'm not sure nals memory is allocated or not? nal = &pkt->nals[pkt->nb_nals]; > > > void *tmp = av_realloc_array(pkt->nals, new_size, sizeof(*pkt->nals)); > > > > if (!tmp) > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 9/9/2019 11:51 PM, Limin Wang wrote: > On Mon, Sep 09, 2019 at 10:53:53PM -0300, James Almer wrote: >> On 9/9/2019 10:18 PM, lance.lmwang@gmail.com wrote: >>> From: Limin Wang <lance.lmwang@gmail.com> >>> >>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> >>> --- >>> libavcodec/h2645_parse.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c >>> index 307e8643e6..403acd2ee1 100644 >>> --- a/libavcodec/h2645_parse.c >>> +++ b/libavcodec/h2645_parse.c >>> @@ -454,7 +454,7 @@ int ff_h2645_packet_split(H2645Packet *pkt, const uint8_t *buf, int length, >>> } >>> >>> if (pkt->nals_allocated < pkt->nb_nals + 1) { >>> - int new_size = pkt->nals_allocated + 1; >>> + int new_size = pkt->nb_nals + 1; >> >> No. nals_allocated and nb_nals are two purposely separate values in >> order to reuse the H2645Packet struct. One keeps track of the allocated >> size of the NAL buffer, whereas the other keeps track of the amount of >> NALs used by the current packet. That's why the check immediately above >> this line makes sure the amount of NALs in the current packet fit in the >> current size of the NAL buffer, and grows/reallocates it otherwise. > > Thanks for the detail information about the two field. Below is my assumation > as I haven't such sample to test such case. > Assuming nals_allocated is 1, nb_nals is 3 That can't happen. nb_nals is always set to zero at the start of ff_h2645_packet_split(), and is increased by one every time a NAL is finished parsing. It's guaranteed to always be less or equal than nals_allocated by the above check. , then the current code will reallocate > new_size with 2, But the following code will use the nb_nals to access pkt->nals, > I'm not sure nals memory is allocated or not? > nal = &pkt->nals[pkt->nb_nals]; > > >> >>> void *tmp = av_realloc_array(pkt->nals, new_size, sizeof(*pkt->nals)); >>> >>> if (!tmp) >>> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Tue, Sep 10, 2019 at 12:03:14AM -0300, James Almer wrote: > On 9/9/2019 11:51 PM, Limin Wang wrote: > > On Mon, Sep 09, 2019 at 10:53:53PM -0300, James Almer wrote: > >> On 9/9/2019 10:18 PM, lance.lmwang@gmail.com wrote: > >>> From: Limin Wang <lance.lmwang@gmail.com> > >>> > >>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > >>> --- > >>> libavcodec/h2645_parse.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c > >>> index 307e8643e6..403acd2ee1 100644 > >>> --- a/libavcodec/h2645_parse.c > >>> +++ b/libavcodec/h2645_parse.c > >>> @@ -454,7 +454,7 @@ int ff_h2645_packet_split(H2645Packet *pkt, const uint8_t *buf, int length, > >>> } > >>> > >>> if (pkt->nals_allocated < pkt->nb_nals + 1) { > >>> - int new_size = pkt->nals_allocated + 1; > >>> + int new_size = pkt->nb_nals + 1; > >> > >> No. nals_allocated and nb_nals are two purposely separate values in > >> order to reuse the H2645Packet struct. One keeps track of the allocated > >> size of the NAL buffer, whereas the other keeps track of the amount of > >> NALs used by the current packet. That's why the check immediately above > >> this line makes sure the amount of NALs in the current packet fit in the > >> current size of the NAL buffer, and grows/reallocates it otherwise. > > > > Thanks for the detail information about the two field. Below is my assumation > > as I haven't such sample to test such case. > > Assuming nals_allocated is 1, nb_nals is 3 > > That can't happen. nb_nals is always set to zero at the start of > ff_h2645_packet_split(), and is increased by one every time a NAL is > finished parsing. It's guaranteed to always be less or equal than > nals_allocated by the above check. > > , then the current code will reallocate Thanks, it's clear now. I have small change to get better code readiablity(which I think) > > new_size with 2, But the following code will use the nb_nals to access pkt->nals, > > I'm not sure nals memory is allocated or not? > > nal = &pkt->nals[pkt->nb_nals]; > > > > > >> > >>> void *tmp = av_realloc_array(pkt->nals, new_size, sizeof(*pkt->nals)); > >>> > >>> if (!tmp) > >>> > >> > >> _______________________________________________ > >> ffmpeg-devel mailing list > >> ffmpeg-devel@ffmpeg.org > >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > >> To unsubscribe, visit link above, or email > >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c index 307e8643e6..403acd2ee1 100644 --- a/libavcodec/h2645_parse.c +++ b/libavcodec/h2645_parse.c @@ -454,7 +454,7 @@ int ff_h2645_packet_split(H2645Packet *pkt, const uint8_t *buf, int length, } if (pkt->nals_allocated < pkt->nb_nals + 1) { - int new_size = pkt->nals_allocated + 1; + int new_size = pkt->nb_nals + 1; void *tmp = av_realloc_array(pkt->nals, new_size, sizeof(*pkt->nals)); if (!tmp)