Hi Andreas,
no objections for the patchset as a whole.
Just for the first in the series I have some questions.
The commit message is:
avformat/au: Store strings instead of pointers to strings in array
This tells the what, but not the why.
On 2020-07-17 13:14 +0200, Andreas Rheinhardt wrote:
> Andreas Rheinhardt:
> > Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
> > ---
> > libavformat/au.c | 13 ++++++-------
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavformat/au.c b/libavformat/au.c
> > index f92863e400..b419c9ed95 100644
> > --- a/libavformat/au.c
> > +++ b/libavformat/au.c
> > @@ -68,13 +68,13 @@ static int au_probe(const AVProbeData *p)
> >
> > static int au_read_annotation(AVFormatContext *s, int size)
> > {
> > - static const char * keys[] = {
> > + static const char keys[][7] = {
> > "title",
> > "artist",
> > "album",
> > "track",
> > "genre",
> > - NULL };
> > + };
Sorry, I couldn't test myself but wouldn't just
static const char * keys[] = {
"title",
"artist",
"album",
"track",
"genre",
};
have been the better choice with the same benefits?
I'm not sure without looking up the C standard and writing some
little test programs. From my guts I would say it's equivalent,
but the syntax is more convenient this way.
That's also another issue with the commit message, since I don't
think it is true that in your version strings are stored in the
array. No offense intended and I sure might be mistaken.
> > AVIOContext *pb = s->pb;
> > enum { PARSE_KEY, PARSE_VALUE, PARSE_FINISHED } state = PARSE_KEY;
> > char c;
> > @@ -107,7 +107,7 @@ static int au_read_annotation(AVFormatContext *s, int
> > size)
> > av_log(s, AV_LOG_ERROR, "Memory error while parsing AU
> > metadata.\n");
> > } else {
> > av_bprint_init(&bprint, 64, AV_BPRINT_SIZE_UNLIMITED);
> > - for (i = 0; keys[i] != NULL && key != NULL; i++) {
> > + for (i = 0; i < FF_ARRAY_ELEMS(keys) && key != NULL;
> > i++) {
> > if (av_strcasecmp(keys[i], key) == 0) {
> > av_dict_set(&(s->metadata), keys[i], value,
> > AV_DICT_DONT_STRDUP_VAL);
> > av_freep(&key);
> > @@ -243,14 +243,13 @@ typedef struct AUContext {
> >
> > static int au_get_annotations(AVFormatContext *s, char **buffer)
> > {
> > - static const char * keys[] = {
> > + static const char keys[][7] = {
> > "Title",
> > "Artist",
> > "Album",
> > "Track",
> > "Genre",
> > - NULL };
> > - int i;
> > + };
> > int cnt = 0;
> > AVDictionary *m = s->metadata;
> > AVDictionaryEntry *t = NULL;
> > @@ -258,7 +257,7 @@ static int au_get_annotations(AVFormatContext *s, char
> > **buffer)
> >
> > av_bprint_init(&bprint, 64, AV_BPRINT_SIZE_UNLIMITED);
> >
> > - for (i = 0; keys[i] != NULL; i++) {
> > + for (int i = 0; i < FF_ARRAY_ELEMS(keys); i++) {
> > t = av_dict_get(m, keys[i], NULL, 0);
> > if (t != NULL) {
> > if (cnt++)
> >
> Will apply this patchset tomorrow unless there are objections.
No problem if I was to late to reply. Just got to read this mail now.
Best regards,
Alexander
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
