| Message ID | 1638507433-5005-2-git-send-email-lance.lmwang@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/aviobuf: check if read_packet() exist before read_packet_wrapper() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Fri, 3 Dec 2021, lance.lmwang@gmail.com wrote: > From: Limin Wang <lance.lmwang@gmail.com> > > This is allowed to read fewer bytes than requested. The missing bytes can > be read in the next call. I don't think this is needed, after all we want to read all data, there is no point in reading it in smaller chunks, no? Thanks, Marton > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > libavformat/aviobuf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c > index 14688a2..ebb86e5 100644 > --- a/libavformat/aviobuf.c > +++ b/libavformat/aviobuf.c > @@ -1317,7 +1317,7 @@ int avio_read_to_bprint(AVIOContext *h, AVBPrint *pb, size_t max_size) > int ret; > char buf[1024]; > while (max_size) { > - ret = avio_read(h, buf, FFMIN(max_size, sizeof(buf))); > + ret = avio_read_partial(h, buf, FFMIN(max_size, sizeof(buf))); > if (ret == AVERROR_EOF) > return 0; > if (ret <= 0) > -- > 1.8.3.1 > > _______________________________________________ > 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 Fri, Dec 03, 2021 at 10:47:00AM +0100, Marton Balint wrote: > > > On Fri, 3 Dec 2021, lance.lmwang@gmail.com wrote: > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > This is allowed to read fewer bytes than requested. The missing bytes can > > be read in the next call. > > I don't think this is needed, after all we want to read all data, there is > no point in reading it in smaller chunks, no? Now the buf is 1024, in case I'm using sdp file, most of my simple testing sdp is about 300 byte, so I think it's preferable to use avio_read_partial and let it return directly. as we don't expect to read 1024 at all. > > Thanks, > Marton > > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > libavformat/aviobuf.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c > > index 14688a2..ebb86e5 100644 > > --- a/libavformat/aviobuf.c > > +++ b/libavformat/aviobuf.c > > @@ -1317,7 +1317,7 @@ int avio_read_to_bprint(AVIOContext *h, AVBPrint *pb, size_t max_size) > > int ret; > > char buf[1024]; > > while (max_size) { > > - ret = avio_read(h, buf, FFMIN(max_size, sizeof(buf))); > > + ret = avio_read_partial(h, buf, FFMIN(max_size, sizeof(buf))); > > if (ret == AVERROR_EOF) > > return 0; > > if (ret <= 0) > > -- > > 1.8.3.1 > > > > _______________________________________________ > > 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 Fri, 3 Dec 2021, lance.lmwang@gmail.com wrote: > On Fri, Dec 03, 2021 at 10:47:00AM +0100, Marton Balint wrote: >> >> >> On Fri, 3 Dec 2021, lance.lmwang@gmail.com wrote: >> >>> From: Limin Wang <lance.lmwang@gmail.com> >>> >>> This is allowed to read fewer bytes than requested. The missing bytes can >>> be read in the next call. >> >> I don't think this is needed, after all we want to read all data, there is >> no point in reading it in smaller chunks, no? > > Now the buf is 1024, in case I'm using sdp file, most of my simple testing sdp is about 300 > byte, so I think it's preferable to use avio_read_partial and let it return directly. as we > don't expect to read 1024 at all. I still don't get it. 1024 is the max you will read, nothing will happen if the file or the underlying buffer is less than that, so I don't understand when you write you want to read it "directly", or what benefit it might hold. avio_read_partial() can be used to reduce latency, but since you want to read fixed amount of bytes in avio_read_to_bprint, there is no reason to use that. Regards, Marton > >> >> Thanks, >> Marton >> >>> >>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> >>> --- >>> libavformat/aviobuf.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c >>> index 14688a2..ebb86e5 100644 >>> --- a/libavformat/aviobuf.c >>> +++ b/libavformat/aviobuf.c >>> @@ -1317,7 +1317,7 @@ int avio_read_to_bprint(AVIOContext *h, AVBPrint *pb, size_t max_size) >>> int ret; >>> char buf[1024]; >>> while (max_size) { >>> - ret = avio_read(h, buf, FFMIN(max_size, sizeof(buf))); >>> + ret = avio_read_partial(h, buf, FFMIN(max_size, sizeof(buf))); >>> if (ret == AVERROR_EOF) >>> return 0; >>> if (ret <= 0) >>> -- >>> 1.8.3.1 >>> >>> _______________________________________________ >>> 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". > > -- > Thanks, > Limin Wang > _______________________________________________ > 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 Fri, 3 Dec 2021, lance.lmwang@gmail.com wrote: > On Fri, Dec 03, 2021 at 10:47:00AM +0100, Marton Balint wrote: >> >> >> On Fri, 3 Dec 2021, lance.lmwang@gmail.com wrote: >> >>> From: Limin Wang <lance.lmwang@gmail.com> >>> >>> This is allowed to read fewer bytes than requested. The missing bytes can >>> be read in the next call. >> >> I don't think this is needed, after all we want to read all data, there is >> no point in reading it in smaller chunks, no? > > Now the buf is 1024, in case I'm using sdp file, most of my simple > testing sdp is about 300 byte, so I think it's preferable to use > avio_read_partial and let it return directly. as we don't expect to read > 1024 at all. No, that's not how it works. avio_read_to_bprint loops until it either has read max_size bytes, or until it reaches EOF. If you use avio_read_partial, it would only loop more times, but it would take the exact same amount of time. If you read a file from disk, once it has read the 300 bytes of actual data, and reach EOF, it returns directly. avio_read_partial is only relevant for reading data e.g. over a network, where there's more data following, but you want to consume and process the initially received data before waiting for more. But with avio_read_partial it doesn't return until all data has been received anyway. // Martin
On Fri, Dec 03, 2021 at 01:59:16PM +0200, Martin Storsjö wrote: > On Fri, 3 Dec 2021, lance.lmwang@gmail.com wrote: > > > On Fri, Dec 03, 2021 at 10:47:00AM +0100, Marton Balint wrote: > > > > > > > > > On Fri, 3 Dec 2021, lance.lmwang@gmail.com wrote: > > > > > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > > > > > This is allowed to read fewer bytes than requested. The missing bytes can > > > > be read in the next call. > > > > > > I don't think this is needed, after all we want to read all data, there is > > > no point in reading it in smaller chunks, no? > > > > Now the buf is 1024, in case I'm using sdp file, most of my simple > > testing sdp is about 300 byte, so I think it's preferable to use > > avio_read_partial and let it return directly. as we don't expect to read > > 1024 at all. > > No, that's not how it works. > > avio_read_to_bprint loops until it either has read max_size bytes, or until > it reaches EOF. If you use avio_read_partial, it would only loop more times, > but it would take the exact same amount of time. > > If you read a file from disk, once it has read the 300 bytes of actual data, > and reach EOF, it returns directly. > > avio_read_partial is only relevant for reading data e.g. over a network, > where there's more data following, but you want to consume and process the > initially received data before waiting for more. But with avio_read_partial > it doesn't return until all data has been received anyway. Thanks for the comments, it seems that I misunderstand the usage of avio_read_partial, please ignore the patch then. > > // Martin > > > > _______________________________________________ > 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/libavformat/aviobuf.c b/libavformat/aviobuf.c index 14688a2..ebb86e5 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -1317,7 +1317,7 @@ int avio_read_to_bprint(AVIOContext *h, AVBPrint *pb, size_t max_size) int ret; char buf[1024]; while (max_size) { - ret = avio_read(h, buf, FFMIN(max_size, sizeof(buf))); + ret = avio_read_partial(h, buf, FFMIN(max_size, sizeof(buf))); if (ret == AVERROR_EOF) return 0; if (ret <= 0)