[FFmpeg-devel] [PATCH 03/18] avformat/id3v2: allow ID3 parsing without AVFormatContext

[Michael Niedermayer]

On Tue, Dec 31, 2013 at 12:48:21AM +0200, Anssi Hannula wrote:
> 30.12.2013 18:23, Michael Niedermayer kirjoitti:
> > On Mon, Dec 30, 2013 at 01:14:17PM +0200, Anssi Hannula wrote:
> >> Add ff_id3v2_read_dict() for parsing without AVFormatContext, but
> >> instead with AVIOContext and AVDictionary.
> >>
> >> Chapter parsing is the only functionality that actually needs
> >> AVFormatContext, and AFAICS it should be modified to write the data to
> >> ID3v2ExtraMeta first, from where it can be implanted to AVFormatContext
> >> (like it is done with read_apic() and ff_id3v2_parse_apic()). That is
> >> outside the scope of this patch, though, so AVFormatContext parameter is
> >> still kept in a few more codepaths than I would prefer.
> >>
> >> Signed-off-by: Anssi Hannula <anssi.hannula at iki.fi>
> >> ---
> >>
> >> This causes a lot of av_logs with NULL context, does it matter and if
> >> so, how should it be solved?
> >>
> >>  libavformat/id3v2.c | 160 ++++++++++++++++++++++++++++++----------------------
> >>  libavformat/id3v2.h |  16 +++++-
> >>  2 files changed, 106 insertions(+), 70 deletions(-)
> >>
> >> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> >> index 3eb368e..0ff2df0 100644
> >> --- a/libavformat/id3v2.c
> >> +++ b/libavformat/id3v2.c
> >> @@ -193,7 +193,7 @@ static void free_geobtag(void *obj)
> >>   * actually read.
> >>   * @returns 0 if no error occurred, dst is uninitialized on error
> >>   */
> >> -static int decode_str(AVFormatContext *s, AVIOContext *pb, int encoding,
> >> +static int decode_str(AVIOContext *pb, int encoding,
> >>                        uint8_t **dst, int *maxread)
> >>  {
> >>      int ret;
> >> @@ -204,7 +204,7 @@ static int decode_str(AVFormatContext *s, AVIOContext *pb, int encoding,
> >>      AVIOContext *dynbuf;
> >>  
> >>      if ((ret = avio_open_dyn_buf(&dynbuf)) < 0) {
> >> -        av_log(s, AV_LOG_ERROR, "Error opening memory stream\n");
> >> +        av_log(NULL, AV_LOG_ERROR, "Error opening memory stream\n");
> > 
> > some (void*) context for av_log() could be kept, this would also
> > make the patch smaller
> 
> Yeah, I could keep the otherwise unneeded AVFormatContext parameters
> around just for av_logs if that is preferred (though it can be NULL,
> which is the reason I dropped it completely to avoid any new code
> relying on it).

I think especially for error messages some "context" is important
otherwise the user could not know from which stream they come if
there are multiple possibilities


> 
> Will do so unless you say otherwise.
> 
> -- 
> Anssi Hannula
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131230/2d1e1f39/attachment.asc>