[FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *

[Martin Storsjö]

On Tue, 19 Apr 2022, Marton Balint wrote:

>
>
> On Sat, 16 Apr 2022, Martin Storsjö wrote:
>
>> On Fri, 15 Apr 2022, Tristan Matthews wrote:
>>
>>>  This avoids having to do one pass to calculate the full length to 
> allocate
>>>  followed by a second pass to actually append values.
>>>  ---
>>>  libavformat/librtmp.c | 124 +++++++++++-------------------------------
>>>  1 file changed, 33 insertions(+), 91 deletions(-)
>>>
>>>  diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
>>>  index 43013e46e0..b7e9fc81cf 100644
>>>  --- a/libavformat/librtmp.c
>>>  +++ b/libavformat/librtmp.c
>>>  @@ -25,6 +25,7 @@
>>>   */
>>>
>>>  #include "libavutil/avstring.h"
>>>  +#include "libavutil/bprint.h"
>>>  #include "libavutil/mathematics.h"
>>>  #include "libavutil/opt.h"
>>>  #include "avformat.h"
>>>  @@ -38,6 +39,7 @@
>>>
>>>  typedef struct LibRTMPContext {
>>>      const AVClass *class;
>>>  +    AVBPrint filename;
>>>      RTMP rtmp;
>>>      char *app;
>>>      char *conn;
>>>  @@ -50,7 +52,6 @@ typedef struct LibRTMPContext {
>>>      char *pageurl;
>>>      char *client_buffer_time;
>>>      int live;
>>>  -    char *temp_filename;
>>>      int buffer_size;
>>>  } LibRTMPContext;
>>
>> This looks ok to me.
>>
>> (I guess it also could be considered viable to not store the AVBprint in 
> the 
>> context but detach an allocated string instead; that would decrease the 
>> persistent allocation size a little, at the cost of some more copying, but 
>> the difference is probably negligible, so this probably is sensible as is.)
>
> Agreed.
>
>>
>> Let's still wait for Marton's review too.
>
> LGTM as well. Thanks.

Pushed now then.

// Martin