| Message ID | 20200504182250.26141-5-andreas.rheinhardt@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,01/10] libavformat/nutenc: Remove redundant function parameter | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Mon, May 04, 2020 at 08:22:45PM +0200, Andreas Rheinhardt wrote: > It allows to add analogous functions using e.g. the bytestream API > instead of using an AVIOContext. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavformat/nutenc.c | 49 ++++++++++++++++++++++++-------------------- > 1 file changed, 27 insertions(+), 22 deletions(-) It certainly is possible to use macros to make the put functions work with 2 unrelated APIs. But pretty it is not, why do we need 2 APIs ? thx [...]
Michael Niedermayer: > On Mon, May 04, 2020 at 08:22:45PM +0200, Andreas Rheinhardt wrote: >> It allows to add analogous functions using e.g. the bytestream API >> instead of using an AVIOContext. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >> --- >> libavformat/nutenc.c | 49 ++++++++++++++++++++++++-------------------- >> 1 file changed, 27 insertions(+), 22 deletions(-) > > It certainly is possible to use macros to make the put functions work > with 2 unrelated APIs. > But pretty it is not, why do we need 2 APIs ? > This is done in preparation for patch 6/10 [1] that separates the several variable-length elements that precede the main content (or in case of the syncpoint structure: that is the complete content). By putting them in a separate buffer one can eliminate one level of dynamic buffers. And given that these little buffers are naturally given as buffers, using the bytestream API seems the easiest and nicest way. While one could of course achieve the same by wrapping these little buffers in an AVIOContext via ffio_init_context(), I don't consider this pretty. - Andreas [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-May/262109.html
On Sat, May 09, 2020 at 06:04:13AM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > On Mon, May 04, 2020 at 08:22:45PM +0200, Andreas Rheinhardt wrote: > >> It allows to add analogous functions using e.g. the bytestream API > >> instead of using an AVIOContext. > >> > >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > >> --- > >> libavformat/nutenc.c | 49 ++++++++++++++++++++++++-------------------- > >> 1 file changed, 27 insertions(+), 22 deletions(-) > > > > It certainly is possible to use macros to make the put functions work > > with 2 unrelated APIs. > > But pretty it is not, why do we need 2 APIs ? > > > This is done in preparation for patch 6/10 [1] that separates the > several variable-length elements that precede the main content (or in > case of the syncpoint structure: that is the complete content). By > putting them in a separate buffer one can eliminate one level of dynamic > buffers. And given that these little buffers are naturally given as > buffers, using the bytestream API seems the easiest and nicest way. > > While one could of course achieve the same by wrapping these little > buffers in an AVIOContext via ffio_init_context(), I don't consider this > pretty. I was hoping that my mildly stupid question would lead to some prettier solution. as that failed, teh patch is ok thx [...]
diff --git a/libavformat/nutenc.c b/libavformat/nutenc.c index ca433038d4..a75d9282fe 100644 --- a/libavformat/nutenc.c +++ b/libavformat/nutenc.c @@ -290,25 +290,35 @@ static int get_v_length(uint64_t val) return i; } -/** - * Put val using a variable number of bytes. - */ -static void put_v(AVIOContext *bc, uint64_t val) -{ - int i = get_v_length(val); - - while (--i > 0) - avio_w8(bc, 128 | (uint8_t)(val >> (7*i))); - - avio_w8(bc, val & 127); +#define PUT_FUNCTIONS(type, write, suffix) \ +/** \ + * Put val using a variable number of bytes. \ + */ \ +static void put_v##suffix(type dst, uint64_t val) \ +{ \ + int i = get_v_length(val); \ + \ + while (--i > 0) \ + write(dst, 128 | (uint8_t)(val >> (7*i))); \ + \ + write(dst, val & 127); \ +} \ + \ +static void put_tt##suffix(NUTContext *nut, AVRational *time_base, \ + type dst, uint64_t val) \ +{ \ + val *= nut->time_base_count; \ + val += time_base - nut->time_base; \ + put_v##suffix(dst, val); \ +} \ + \ +static void put_s##suffix(type dst, int64_t val) \ +{ \ + put_v##suffix(dst, 2 * FFABS(val) - (val > 0)); \ } -static void put_tt(NUTContext *nut, AVRational *time_base, AVIOContext *bc, uint64_t val) -{ - val *= nut->time_base_count; - val += time_base - nut->time_base; - put_v(bc, val); -} +PUT_FUNCTIONS(AVIOContext *, avio_w8,) + /** * Store a string as vb. */ @@ -320,11 +330,6 @@ static void put_str(AVIOContext *bc, const char *string) avio_write(bc, string, len); } -static void put_s(AVIOContext *bc, int64_t val) -{ - put_v(bc, 2 * FFABS(val) - (val > 0)); -} - static void put_packet(NUTContext *nut, AVIOContext *bc, AVIOContext *dyn_bc, uint64_t startcode) {
It allows to add analogous functions using e.g. the bytestream API instead of using an AVIOContext. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavformat/nutenc.c | 49 ++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 22 deletions(-)