[libav-devel] [PATCH] ismindex: produce .ismf file

Martin Storsjö martin at martin.st
Thu Sep 4 13:10:18 CEST 2014


On Thu, 4 Sep 2014, Mika Raento wrote:

> This is a non-standard file that maps the MSS segment names to offsets
> in the ISMV file. This can be used to build a custom MSS streaming
> server without splitting the ISMV into separate files.
> ---
> tools/ismindex.c | 115 +++++++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 95 insertions(+), 20 deletions(-)

Ok, now this is quite close. Sorry that I have a few more comments that I 
didn't notice earlier...

> diff --git a/tools/ismindex.c b/tools/ismindex.c
> index a6a9763..85b520f 100644
> --- a/tools/ismindex.c
> +++ b/tools/ismindex.c
> +
> static int write_fragments(struct Tracks *tracks, int start_index,
> -                           AVIOContext *in, const char *output_prefix)
> +                           AVIOContext *in, const char *basename,
> +                           int split, int ismf, const char* output_prefix)
> {
> -    char dirname[2048], filename[2048];
> -    int i, j;
> +    char dirname[2048], filename[2048], idxname[2048];
> +    int i, j, ret, fragment_ret;
> +    FILE* out;
>
> +    out = NULL;
> +    ret = 0;

These initializations can be merged with the declaration, saving a few 
lines

> +    if (ismf) {
> +        snprintf(idxname, sizeof(idxname), "%s%s.ismf", output_prefix, basename);
> +        out = fopen(idxname, "w");
> +        if (!out) {
> +            ret = AVERROR(errno);
> +            perror(idxname);
> +            goto fail;
> +        }
> +    }
>     for (i = start_index; i < tracks->nb_tracks; i++) {
>         struct Track *track = tracks->tracks[i];
>         const char *type    = track->is_video ? "video" : "audio";
>         snprintf(dirname, sizeof(dirname), "%sQualityLevels(%d)", output_prefix, track->bitrate);
> -        mkdir(dirname, 0777);
> +        if (split) {
> +            if (mkdir(dirname, 0777) == -1) {
> +                ret = AVERROR(errno);
> +                perror(dirname);
> +                goto fail;
> +            }
> +        }

Here I actually intentionally ignored errors from mkdir before. Especially 
when testing these tools, it's quite handy to be able to overwrite files 
in place, but mkdir fails if the directory already exists. So here (and 
similarly in libavformat/smoothstreamingenc.c), if mkdir fails we just 
ignore it - we need to check if creating the files worked anyway.

(The downside is that if creating the directory failed, the error messages 
are slightly more confusing. The ideal solution would be to use stat or 
something similar to check whether the directory already exists, but then 
we end up with more function calls that might not be fully portable etc, 
so I've kept it simple.)

>         for (j = 0; j < track->chunks; j++) {
>             snprintf(filename, sizeof(filename), "%s/Fragments(%s=%"PRId64")",
>                      dirname, type, track->offsets[j].time);
>             avio_seek(in, track->offsets[j].offset, SEEK_SET);
> -            write_fragment(filename, in);
> +            if (ismf) {
> +                fprintf(out, "%s %"PRId64, filename, avio_tell(in));
> +            }
> +            if (split) {
> +                fragment_ret = write_fragment(filename, in);
> +            } else  {
> +                fragment_ret = skip_fragment(in);
> +            }
> +            if (ismf) {
> +                fprintf(out, " %"PRId64"\n", avio_tell(in));
> +            }

I think this would be more readable without the extra braces for the 
single-line conditions - at least it would keep the number of lines 
down...

> +            if (fragment_ret != 0) {
> +                fprintf(stderr, "failed fragment %d\n", j);
> +                ret = fragment_ret;
> +            }

Technically this is also a behaviour change from before - previously these 
errors were ignored. But this was ignored more due to lazyness than 
intentionally, so I guess it's ok to add it here.

>         }
>     }
> -    return 0;
> +fail:
> +    if (out)
> +        fclose(out);
> +    return ret;
> }
>
> static int read_tfra(struct Tracks *tracks, int start_index, AVIOContext *f)
> @@ -217,7 +285,8 @@ fail:
> }
>
> static int read_mfra(struct Tracks *tracks, int start_index,
> -                     const char *file, int split, const char *output_prefix)
> +                     const char *file, int split, int ismf,
> +                     const char *basename, const char* output_prefix)
> {
>     int err = 0;
>     const char* err_str = "";
> @@ -243,8 +312,9 @@ static int read_mfra(struct Tracks *tracks, int start_index,
>         /* Empty */
>     }
>
> -    if (split)
> -        write_fragments(tracks, start_index, f, output_prefix);
> +    err = write_fragments(tracks, start_index, f, basename, split, ismf,
> +                          output_prefix);
> +    err_str = "error in write_fragments";

Wouldn't it make more sense to keep the condition, but expand it into "if 
(split || ismf)"?

// Martin


More information about the libav-devel mailing list