On Mon, Aug 08, 2016 at 11:45:38AM -0700, Carlos Fernandez Sanz wrote: [...] > @@ -349,11 +359,25 @@ static int hls_append_segment(struct AVFormatContext > *s, HLSContext *hls, double > else > en->sub_filename[0] = '\0'; > > - en->duration = duration; > - en->pos = pos; > - en->size = size; > + en->duration = duration; > + en->pos = pos; > + en->event = event; > + en->size = size; > + en->start_pts = start_pts; > en->next = NULL;
this reindention is inconsistent, also you can if you like keep
reindentions in a seperate patch. But if done they should be consistent
>
> + if (hls->scte_iface) {
> + if (hls->scte_iface->event_out == EVENT_OUT_CONT) {
> + en->adv_count = hls->scte_iface->adv_count;
> + hls->scte_iface->adv_count++;
> + en->out = hls->scte_iface->event_out;
> + } else {
> + hls->scte_iface->adv_count = 0;
> + en->out = hls->scte_iface->event_out;
> + }
> + }
> +
> +
> if (hls->key_info_file) {
> av_strlcpy(en->key_uri, hls->key_uri, sizeof(en->key_uri));
> av_strlcpy(en->iv_string, hls->iv_string, sizeof(en->iv_string));
> @@ -475,9 +499,23 @@ static int hls_window(AVFormatContext *s, int last)
> if (hls->flags & HLS_SINGLE_FILE)
> avio_printf(out, "#EXT-X-BYTERANGE:%"PRIi64"@%"PRIi64"\n",
> en->size, en->pos);
> - if (hls->baseurl)
> - avio_printf(out, "%s", hls->baseurl);
> - avio_printf(out, "%s\n", en->filename);
> + if (hls->scte_iface && (en->event || en->out) ) {
> + char *str;
> + char fname[1024] = "";
> + if (hls->adv_filename) {
> + str = hls->scte_iface->get_hls_string(hls->scte_iface,
> en->event, hls->adv_filename, en->out, en->adv_count, en->start_pts);
> + } else {
> + if (hls->baseurl)
> + strncat(fname, hls->baseurl, 1024);
> + strncat(fname, en->filename, 1024);
duplicate hardcoded sizes, these should use sizeof()
> + str = hls->scte_iface->get_hls_string(hls->scte_iface,
> en->event, fname, en->out, -1, en->start_pts);
> + }
> + avio_printf(out, "%s", str);
> + } else {
> + if (hls->baseurl)
> + avio_printf(out, "%s", hls->baseurl);
> + avio_printf(out, "%s\n", en->filename);
> + }
> }
>
> if (last && (hls->flags & HLS_OMIT_ENDLIST)==0)
> @@ -502,9 +540,15 @@ static int hls_window(AVFormatContext *s, int last)
> if (hls->flags & HLS_SINGLE_FILE)
> avio_printf(sub_out,
> "#EXT-X-BYTERANGE:%"PRIi64"@%"PRIi64"\n",
> en->size, en->pos);
> - if (hls->baseurl)
> - avio_printf(sub_out, "%s", hls->baseurl);
> - avio_printf(sub_out, "%s\n", en->sub_filename);
> + if (hls->scte_iface && (en->event || en->out) ) {
> + char *str = hls->scte_iface->get_hls_string(hls->scte_iface,
> en->event, hls->adv_subfilename, en->out, en->adv_count, en->pos);
> + avio_printf(out, "%s", str);
> + } else {
> + if (hls->baseurl)
> + avio_printf(out, "%s", hls->baseurl);
> + avio_printf(sub_out, "%s\n", en->sub_filename);
> + }
> +
> }
>
> if (last)
> @@ -647,6 +691,7 @@ static int hls_write_header(AVFormatContext *s)
> AVDictionary *options = NULL;
> int basename_size;
> int vtt_basename_size;
> + int ts_index = 0;
>
> hls->sequence = hls->start_sequence;
> hls->recording_time = hls->time * AV_TIME_BASE;
> @@ -763,19 +808,21 @@ static int hls_write_header(AVFormatContext *s)
> goto fail;
> }
> //av_assert0(s->nb_streams == hls->avf->nb_streams);
> - for (i = 0; i < s->nb_streams; i++) {
> + for (ts_index = 0, i = 0; i < s->nb_streams; i++) {
> AVStream *inner_st;
> AVStream *outer_st = s->streams[i];
> - if (outer_st->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE)
> - inner_st = hls->avf->streams[i];
> - else if (hls->vtt_avf)
> + if (hls->vtt_avf && outer_st->codecpar->codec_type ==
> AVMEDIA_TYPE_SUBTITLE) {
> inner_st = hls->vtt_avf->streams[0];
> - else {
> - /* We have a subtitle stream, when the user does not want one */
> - inner_st = NULL;
> + avpriv_set_pts_info(outer_st, inner_st->pts_wrap_bits,
> inner_st->time_base.num, inner_st->time_base.den);
> + } else if (outer_st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
> + inner_st = hls->avf->streams[ts_index];
> + hls->scte_iface = ff_alloc_scte35_parser(hls,
> outer_st->time_base);
missing failure check
[...]
> +static char* get_hls_string(struct scte_35_interface *iface, struct
> scte_35_event *event,
> + const char *filename, int out_state, int seg_count, int64_t
> pos)
> +{
> + int ret;
> + av_bprint_clear(&iface->avbstr);
> + if (out_state == EVENT_IN ) {
> + av_bprintf(&iface->avbstr, "#EXT-OATCLS-SCTE35:%s\n",
> iface->pkt_base64);
> + av_bprintf(&iface->avbstr, "#EXT-X-CUE-IN\n");
> + av_bprintf(&iface->avbstr, "#EXT-X-DISCONTINUITY\n");
> + } else if (out_state == EVENT_OUT) {
> + if (event)
> + {
inconsistent { placement
> + av_bprintf(&iface->avbstr, "#EXT-OATCLS-SCTE35:%s\n",
> iface->pkt_base64);
> + if(event->duration != AV_NOPTS_VALUE) {
> + int64_t dur = ceil(((double)event->duration *
> iface->timebase.num) /iface->timebase.den);
> + av_bprintf(&iface->avbstr, "#EXT-X-CUE-OUT:%"PRIu64"\n",
> dur);
> + } else {
> + av_bprintf(&iface->avbstr, "#EXT-X-CUE-OUT\n");
> + }
> + av_bprintf(&iface->avbstr, "#EXT-X-DISCONTINUITY\n");
> + }
> + } else if (out_state == EVENT_OUT_CONT) {
> + if(event && event->duration != AV_NOPTS_VALUE) {
> + int64_t dur = ceil(((double)event->duration *
> iface->timebase.num) /iface->timebase.den);
> + int64_t elapsed_time = ceil(((double)pos * iface->timebase.num)
> /iface->timebase.den) - event->out_pts;
> + av_bprintf(&iface->avbstr,
> "#EXT-X-CUE-OUT-CONT:ElapsedTime=%"PRIu64",Duration=%"PRIu64",SCTE35=%s\n",
> + elapsed_time, dur, iface->pkt_base64);
> + } else {
> + av_bprintf(&iface->avbstr, "#EXT-X-CUE-OUT-CONT:SCTE35=%s\n",
> iface->pkt_base64);
> + }
> + }
> + if (seg_count >= 0)
> + av_bprintf(&iface->avbstr, filename, seg_count);
> + else
> + av_bprintf(&iface->avbstr, "%s", filename);
> + av_bprintf(&iface->avbstr, "\n");
> +
> + ret = av_bprint_is_complete(&iface->avbstr);
> + if( ret == 0) {
> + av_log(NULL, AV_LOG_DEBUG, "Out of Memory");
Missing log context, a log context is important so the user or app
know from where a message originated
> + return NULL;
> + }
> +
> + av_log(iface->parent, AV_LOG_DEBUG, "%s", iface->avbstr.str);
> + return iface->avbstr.str;
> +}
> +
> +static struct scte_35_event* alloc_scte35_event(int id)
> +{
> + struct scte_35_event* event = av_malloc(sizeof(struct scte_35_event));
> + event->id = id;
missing malloc failure check
> + event->in_pts = AV_NOPTS_VALUE;
> + event->nearest_in_pts = AV_NOPTS_VALUE;
> + event->out_pts = AV_NOPTS_VALUE;
> + event->lock = 0;
> + event->cancel = 1;
> + event->next = NULL;
> + event->prev = NULL;
av_mallocz() can simplify this
[...]
> +struct scte_35_interface* ff_alloc_scte35_parser(void *parent, AVRational
> timebase)
> +{
> + struct scte_35_interface* iface = av_mallocz(sizeof(struct
> scte_35_interface));
> +
> + iface->parent = parent;
missing malloc failure check
> + iface->timebase = timebase;
> + iface->get_event_pts = get_event_pts;
> + iface->get_event_cache = get_event_cache;
> + av_bprint_init(&iface->avbstr, 0, AV_BPRINT_SIZE_UNLIMITED);
> + iface->get_hls_string = get_hls_string;
> + iface->unref_scte35_event = unref_scte35_event;
> + iface->ref_scte35_event = ref_scte35_event;
> + iface->event_out = EVENT_NONE;
> + iface->prev_event_state = EVENT_NONE;
> + return iface;
> +}
> +
> +void ff_delete_scte35_parser(struct scte_35_interface* iface)
> +{
> + av_freep(&iface);
> +}
> diff --git a/libavformat/scte_35.h b/libavformat/scte_35.h
> new file mode 100644
> index 0000000..35a84c9
> --- /dev/null
> +++ b/libavformat/scte_35.h
> @@ -0,0 +1,76 @@
> +/*
> + * SCTE-35 parser
> + * Copyright (c) 2016 Carlos Fernandez
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +#ifndef AVFORMAT_SCTE_35_H
> +#define AVFORMAT_SCTE_35_H
> +
> +#include "libavutil/bprint.h"
> +
> +struct scte_35_event {
> + int32_t id;
> + uint64_t in_pts;
> + uint64_t nearest_in_pts;
> + uint64_t out_pts;
> + int64_t duration;
> + int64_t start_pos;
> + int cancel;
> + /*if advertisement have already started cancel command can't delete
> advertisement */
> + volatile int lock;
> + volatile int ref_count;
iam not sure what you are trying to do here but the volatile is
very likely not achieving it
can you explain what the volatile is intended to do ?
> + struct scte_35_event *next;
> + struct scte_35_event *prev;
> +};
> +struct scte_35_interface {
> + struct scte_35_event *event_list;
> + char adv_filename[1024];
> + char filename[1024];
> + int event_out;
> + AVRational timebase;
> + int adv_count;
> + struct scte_35_event *cache_event;
> + int prev_event_state;
> + //TODO use AV_BASE64_SIZE to dynamically allocate the array
> + char pkt_base64[1024];
> + /* keep context of its parent for log */
> + void *parent;
> +
> + struct scte_35_event* (*get_event_pts)(struct scte_35_interface *iface,
> uint64_t pts);
> + struct scte_35_event* (*get_event_cache)(struct scte_35_interface
> *iface);
> + /* general purpose str */
> + AVBPrint avbstr;
> + char* (*get_hls_string)(struct scte_35_interface *iface, struct
> scte_35_event *event,
> + const char *adv_filename, int out_state, int seg_count,
> int64_t pos);
> +
> + void (*unref_scte35_event)(struct scte_35_event **event);
> + void (*ref_scte35_event)(struct scte_35_event *event);
These function pointers are only initialized to the same functions
so they wouldnt not be needed if iam not missing anything.
But please explain what the idea here is ?
> +};
> +
> +enum event_state {
> + EVENT_NONE,
> + EVENT_IN,
> + EVENT_OUT,
> + EVENT_OUT_CONT,
> +};
> +
> +int ff_parse_scte35_pkt(struct scte_35_interface *iface, const AVPacket
> *avpkt);
> +
> +struct scte_35_interface* ff_alloc_scte35_parser(void *parent, AVRational
> timebase);
> +void ff_delete_scte35_parser(struct scte_35_interface* iface);
most of the functions and structure fields in the header are lacking
documentation
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
