| Message ID | AS8P250MB074468539EEA8A6D4B52BBA38FF2A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Commit | b67171893d91ee9f2901ef5d91db7ee0ed9848ed |
| Headers | show |
| Series | [FFmpeg-devel,v3] avformat/avio: Don't use incompatible function pointer type for call | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Mon, 11 Sep 2023, Andreas Rheinhardt wrote: > It is undefined behaviour even in cases where it works > (it works because it is only a const uint8_t* vs. uint8_t* difference). > > Instead add a cbuf parameter to pass a const buffer (for writing) > as well as a parameter indicating whether we are reading or writing; > retry_transfer_wrapper() itself then uses the correct function > based upon this information. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > Another approach; IMO the best of the three. What do you think? OK for me, thanks. Marton > > libavformat/avio.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/libavformat/avio.c b/libavformat/avio.c > index b6a192892a..b793a7546c 100644 > --- a/libavformat/avio.c > +++ b/libavformat/avio.c > @@ -348,10 +348,9 @@ fail: > } > > static inline int retry_transfer_wrapper(URLContext *h, uint8_t *buf, > + const uint8_t *cbuf, > int size, int size_min, > - int (*transfer_func)(URLContext *h, > - uint8_t *buf, > - int size)) > + int read) > { > int ret, len; > int fast_retries = 5; > @@ -361,7 +360,8 @@ static inline int retry_transfer_wrapper(URLContext *h, uint8_t *buf, > while (len < size_min) { > if (ff_check_interrupt(&h->interrupt_callback)) > return AVERROR_EXIT; > - ret = transfer_func(h, buf + len, size - len); > + ret = read ? h->prot->url_read (h, buf + len, size - len): > + h->prot->url_write(h, cbuf + len, size - len); > if (ret == AVERROR(EINTR)) > continue; > if (h->flags & AVIO_FLAG_NONBLOCK) > @@ -398,14 +398,14 @@ int ffurl_read2(void *urlcontext, uint8_t *buf, int size) > > if (!(h->flags & AVIO_FLAG_READ)) > return AVERROR(EIO); > - return retry_transfer_wrapper(h, buf, size, 1, h->prot->url_read); > + return retry_transfer_wrapper(h, buf, NULL, size, 1, 1); > } > > int ffurl_read_complete(URLContext *h, unsigned char *buf, int size) > { > if (!(h->flags & AVIO_FLAG_READ)) > return AVERROR(EIO); > - return retry_transfer_wrapper(h, buf, size, size, h->prot->url_read); > + return retry_transfer_wrapper(h, buf, NULL, size, size, 1); > } > > #if FF_API_AVIO_WRITE_NONCONST > @@ -422,9 +422,7 @@ int ffurl_write2(void *urlcontext, const uint8_t *buf, int size) > if (h->max_packet_size && size > h->max_packet_size) > return AVERROR(EIO); > > - return retry_transfer_wrapper(h, (unsigned char *)buf, size, size, > - (int (*)(struct URLContext *, uint8_t *, int)) > - h->prot->url_write); > + return retry_transfer_wrapper(h, NULL, buf, size, size, 0); > } > > int64_t ffurl_seek2(void *urlcontext, int64_t pos, int whence) > -- > 2.34.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". >
Andreas Rheinhardt: > It is undefined behaviour even in cases where it works > (it works because it is only a const uint8_t* vs. uint8_t* difference). > > Instead add a cbuf parameter to pass a const buffer (for writing) > as well as a parameter indicating whether we are reading or writing; > retry_transfer_wrapper() itself then uses the correct function > based upon this information. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > Another approach; IMO the best of the three. What do you think? > > libavformat/avio.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/libavformat/avio.c b/libavformat/avio.c > index b6a192892a..b793a7546c 100644 > --- a/libavformat/avio.c > +++ b/libavformat/avio.c > @@ -348,10 +348,9 @@ fail: > } > > static inline int retry_transfer_wrapper(URLContext *h, uint8_t *buf, > + const uint8_t *cbuf, > int size, int size_min, > - int (*transfer_func)(URLContext *h, > - uint8_t *buf, > - int size)) > + int read) > { > int ret, len; > int fast_retries = 5; > @@ -361,7 +360,8 @@ static inline int retry_transfer_wrapper(URLContext *h, uint8_t *buf, > while (len < size_min) { > if (ff_check_interrupt(&h->interrupt_callback)) > return AVERROR_EXIT; > - ret = transfer_func(h, buf + len, size - len); > + ret = read ? h->prot->url_read (h, buf + len, size - len): > + h->prot->url_write(h, cbuf + len, size - len); > if (ret == AVERROR(EINTR)) > continue; > if (h->flags & AVIO_FLAG_NONBLOCK) > @@ -398,14 +398,14 @@ int ffurl_read2(void *urlcontext, uint8_t *buf, int size) > > if (!(h->flags & AVIO_FLAG_READ)) > return AVERROR(EIO); > - return retry_transfer_wrapper(h, buf, size, 1, h->prot->url_read); > + return retry_transfer_wrapper(h, buf, NULL, size, 1, 1); > } > > int ffurl_read_complete(URLContext *h, unsigned char *buf, int size) > { > if (!(h->flags & AVIO_FLAG_READ)) > return AVERROR(EIO); > - return retry_transfer_wrapper(h, buf, size, size, h->prot->url_read); > + return retry_transfer_wrapper(h, buf, NULL, size, size, 1); > } > > #if FF_API_AVIO_WRITE_NONCONST > @@ -422,9 +422,7 @@ int ffurl_write2(void *urlcontext, const uint8_t *buf, int size) > if (h->max_packet_size && size > h->max_packet_size) > return AVERROR(EIO); > > - return retry_transfer_wrapper(h, (unsigned char *)buf, size, size, > - (int (*)(struct URLContext *, uint8_t *, int)) > - h->prot->url_write); > + return retry_transfer_wrapper(h, NULL, buf, size, size, 0); > } > > int64_t ffurl_seek2(void *urlcontext, int64_t pos, int whence) Will apply this tonight unless there are objections. - Andreas
diff --git a/libavformat/avio.c b/libavformat/avio.c index b6a192892a..b793a7546c 100644 --- a/libavformat/avio.c +++ b/libavformat/avio.c @@ -348,10 +348,9 @@ fail: } static inline int retry_transfer_wrapper(URLContext *h, uint8_t *buf, + const uint8_t *cbuf, int size, int size_min, - int (*transfer_func)(URLContext *h, - uint8_t *buf, - int size)) + int read) { int ret, len; int fast_retries = 5; @@ -361,7 +360,8 @@ static inline int retry_transfer_wrapper(URLContext *h, uint8_t *buf, while (len < size_min) { if (ff_check_interrupt(&h->interrupt_callback)) return AVERROR_EXIT; - ret = transfer_func(h, buf + len, size - len); + ret = read ? h->prot->url_read (h, buf + len, size - len): + h->prot->url_write(h, cbuf + len, size - len); if (ret == AVERROR(EINTR)) continue; if (h->flags & AVIO_FLAG_NONBLOCK) @@ -398,14 +398,14 @@ int ffurl_read2(void *urlcontext, uint8_t *buf, int size) if (!(h->flags & AVIO_FLAG_READ)) return AVERROR(EIO); - return retry_transfer_wrapper(h, buf, size, 1, h->prot->url_read); + return retry_transfer_wrapper(h, buf, NULL, size, 1, 1); } int ffurl_read_complete(URLContext *h, unsigned char *buf, int size) { if (!(h->flags & AVIO_FLAG_READ)) return AVERROR(EIO); - return retry_transfer_wrapper(h, buf, size, size, h->prot->url_read); + return retry_transfer_wrapper(h, buf, NULL, size, size, 1); } #if FF_API_AVIO_WRITE_NONCONST @@ -422,9 +422,7 @@ int ffurl_write2(void *urlcontext, const uint8_t *buf, int size) if (h->max_packet_size && size > h->max_packet_size) return AVERROR(EIO); - return retry_transfer_wrapper(h, (unsigned char *)buf, size, size, - (int (*)(struct URLContext *, uint8_t *, int)) - h->prot->url_write); + return retry_transfer_wrapper(h, NULL, buf, size, size, 0); } int64_t ffurl_seek2(void *urlcontext, int64_t pos, int whence)
It is undefined behaviour even in cases where it works (it works because it is only a const uint8_t* vs. uint8_t* difference). Instead add a cbuf parameter to pass a const buffer (for writing) as well as a parameter indicating whether we are reading or writing; retry_transfer_wrapper() itself then uses the correct function based upon this information. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- Another approach; IMO the best of the three. What do you think? libavformat/avio.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)