[FFmpeg-devel] Fixing rotation metadata override
Hi, here is a fix for issue: https://trac.ffmpeg.org/ticket/4560 Background: since FFmpeg 3.3, it is not possible to force the rotation metadata to 0, when video is being converted. It only work in -c:v copy mode. Can you please have a look at it ? Thanks 0001-Fix-rotation-metadata-override-when-transcoding.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Fixing rotation metadata override
[libx264 @ 0x3cbea80] 8x8 transform intra:53.6% inter:86.2% [libx264 @ 0x3cbea80] coded y,uvDC,uvAC intra: 11.6% 21.3% 1.3% inter: 7.6% 11.0% 0.2% [libx264 @ 0x3cbea80] i16 v,h,dc,p: 41% 24% 11% 23% [libx264 @ 0x3cbea80] i8 v,h,dc,ddl,ddr,vr,hd,vl,hu: 24% 12% 56% 1% 1% 2% 1% 1% 1% [libx264 @ 0x3cbea80] i4 v,h,dc,ddl,ddr,vr,hd,vl,hu: 38% 20% 18% 3% 4% 6% 4% 5% 3% [libx264 @ 0x3cbea80] i8c dc,h,v,p: 64% 17% 18% 1% [libx264 @ 0x3cbea80] Weighted P-Frames: Y:0.0% UV:0.0% [libx264 @ 0x3cbea80] ref P L0: 69.8% 13.2% 13.8% 3.2% [libx264 @ 0x3cbea80] ref B L0: 91.0% 9.0% [libx264 @ 0x3cbea80] kb/s:5739.08 -- The resulting file still has a 90degres rotation: mediainfo out.mp4 | grep -i rot Rotation : 90? Le sam. 13 oct. 2018 à 13:41, Carl Eugen Hoyos a écrit : > 2018-10-13 8:31 GMT+02:00, Gilles : > > > here is a fix for issue: https://trac.ffmpeg.org/ticket/4560 > > This ticket was fixed years ago and the issue is not > reproducible with current FFmpeg git head. > > > Background: since FFmpeg 3.3, it is not possible to force > > the rotation metadata to 0, when video is being converted. > > It only work in -c:v copy mode. > > Sounds like ticket #6370. > > Carl Eugen > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Fixing rotation metadata override
Hi, the one in the first post: https://trac.ffmpeg.org/ticket/4560 Le lun. 15 oct. 2018 à 11:40, Carl Eugen Hoyos a écrit : > 2018-10-15 10:29 GMT+02:00, Gilles : > > > it looks like to me that the issue is still present. > > Which issue? Please fix the commit message accordingly. > > Please do not top-post here, Carl Eugen > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Fixing rotation metadata override
Oh my bad, I had 2 tickets on this subject and the one that is still opened is: https://trac.ffmpeg.org/ticket/6370 Anyway, the issue is still around Le lun. 15 oct. 2018 à 13:27, Carl Eugen Hoyos a écrit : > 2018-10-15 12:11 GMT+02:00, Gilles : > > > the one in the first post: https://trac.ffmpeg.org/ticket/4560 > > Again: > This ticket was fixed years ago and is not reproducible with > current FFmpeg git head. > > Please stop top-posting here, Carl Eugen > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 0/7] using ffmpeg as an RTSP client
Hi, I am using ffmpeg 2.5.3 as an RTSP client and found a few missing pieces, which the following series of patches should address. I used some test files streamed with an RTSP server on the xenomai.org server, the files are available for download by replacing rtsp://xenomai.org:8554 with https://xenomai.org in the URL - support for depacketization of AC3 RTP frames is missing, patch 1 adds it, following https://tools.ietf.org/html/draft-ietf-avt-rtp-ac3-07 it can be tested with: rtsp://xenomai.org:8554/video-tests/big_buck_bunny/BigBuckBunny_DivX_HD720p_ASP.divx (using audio track 2, as audio track 1 uses mp3adu) - support for receiving sub-titles via RTP is missing. live555 uses the T140 RTP packetization for that (RFC 4103), and vlc decodes subrip subtitles sent that way. Patch 2 and 3 implement a compatible solution, meaning that the AV_CODEC_ID_SUBRIP is used for T140 tracks, of course another codec id could be used. These patches can be tested with: rtsp://xenomai.org:8554/video-tests/sintel/sintel-1280-surround.mp4 - support for receiving MP3ADU audio via RTP, following RFC 3119 is missing. Patch 4 modifies the MPEG audio parser to decode MP3ADU headers in MP3ADU frames only when the PARSER_FLAG_COMPLETE_FRAMES is set, because for some reason the parser creates incorrect frames when trying to parse MP3ADU frames fully with the test file I used. Patch 5 implements the depacketizer. These patches can be tested with: rtsp://xenomai.org:8554/video-tests/big_buck_bunny/BigBuckBunny_DivX_HD720p_ASP.divx (this time using audio track 1) - support for reassembly of fragmented AAC frames via RTP is missing. Patch 6 modifies libavformat/rtpdec_mpeg4.c to add that support. This patch can be tested with: rtsp://xenomai.org:8554/video-tests/sintel/sintel-1280-surround.mp4 Without this patch, fragmented AAC frames are silently dropped, which causes underruns in the audio driver when playing the test file. - support for parsing the lang attribute in RTSP tracks is missing. Patch 7 adds it. This patch can be tested with: rtsp://xenomai.org:8554/video-tests/sintel/sintel-1280-surround.mp4 Launching ffprobe with this URL should display the languages of the audio track and subtitle tracks. Thanks in advance for your review. Regards. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/7] avformat/rtsp: allow receiving subtitles via RTP
---
libavformat/rtsp.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index ae62252..77407dd 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -68,10 +68,11 @@
{ "filter_src", "only receive packets from the negotiated peer IP", 0,
AV_OPT_TYPE_CONST, {.i64 = RTSP_FLAG_FILTER_SRC}, 0, 0, DEC, "rtsp_flags" }
#define RTSP_MEDIATYPE_OPTS(name, longname) \
-{ name, longname, OFFSET(media_type_mask), AV_OPT_TYPE_FLAGS, { .i64 = (1
<< (AVMEDIA_TYPE_DATA+1)) - 1 }, INT_MIN, INT_MAX, DEC, "allowed_media_types"
}, \
+{ name, longname, OFFSET(media_type_mask), AV_OPT_TYPE_FLAGS, { .i64 = (1
<< (AVMEDIA_TYPE_SUBTITLE+1)) - 1 }, INT_MIN, INT_MAX, DEC,
"allowed_media_types" }, \
{ "video", "Video", 0, AV_OPT_TYPE_CONST, {.i64 = 1 <<
AVMEDIA_TYPE_VIDEO}, 0, 0, DEC, "allowed_media_types" }, \
{ "audio", "Audio", 0, AV_OPT_TYPE_CONST, {.i64 = 1 <<
AVMEDIA_TYPE_AUDIO}, 0, 0, DEC, "allowed_media_types" }, \
-{ "data", "Data", 0, AV_OPT_TYPE_CONST, {.i64 = 1 << AVMEDIA_TYPE_DATA},
0, 0, DEC, "allowed_media_types" }
+{ "data", "Data", 0, AV_OPT_TYPE_CONST, {.i64 = 1 << AVMEDIA_TYPE_DATA},
0, 0, DEC, "allowed_media_types" }, \
+{ "subtitle", "Subtitle", 0, AV_OPT_TYPE_CONST, {.i64 = 1 <<
AVMEDIA_TYPE_SUBTITLE}, 0, 0, DEC, "allowed_media_types" }
#define RTSP_REORDERING_OPTS() \
{ "reorder_queue_size", "set number of packets to buffer for handling of
reordered packets", OFFSET(reordering_queue_size), AV_OPT_TYPE_INT, { .i64 = -1
}, -1, INT_MAX, DEC }
@@ -399,6 +400,8 @@ static void sdp_parse_line(AVFormatContext *s,
SDPParseState *s1,
codec_type = AVMEDIA_TYPE_VIDEO;
} else if (!strcmp(st_type, "application")) {
codec_type = AVMEDIA_TYPE_DATA;
+} else if (!strcmp(st_type, "text")) {
+codec_type = AVMEDIA_TYPE_SUBTITLE;
}
if (codec_type == AVMEDIA_TYPE_UNKNOWN || !(rt->media_type_mask & (1
<< codec_type))) {
s1->skip_media = 1;
@@ -2368,7 +2371,7 @@ static int rtp_read_header(AVFormatContext *s)
/* sdp_read_header initializes this again */
ff_network_close();
-rt->media_type_mask = (1 << (AVMEDIA_TYPE_DATA+1)) - 1;
+rt->media_type_mask = (1 << (AVMEDIA_TYPE_SUBTITLE+1)) - 1;
ret = sdp_read_header(s);
s->pb = NULL;
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/7] avformat/rtpdec_ac3: add AC3 RTP depacketization
---
libavformat/Makefile | 1 +
libavformat/rtpdec.c | 1 +
libavformat/rtpdec_ac3.c | 166 +++
libavformat/rtpdec_formats.h | 1 +
4 files changed, 169 insertions(+)
create mode 100644 libavformat/rtpdec_ac3.c
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 6bf0761..5acb292 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -30,6 +30,7 @@ OBJS-$(CONFIG_RIFFENC) += riffenc.o
OBJS-$(CONFIG_RTPDEC)+= rdt.o \
rtp.o \
rtpdec.o\
+rtpdec_ac3.o\
rtpdec_amr.o\
rtpdec_asf.o\
rtpdec_g726.o \
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index 33205aa..f5557d8 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -66,6 +66,7 @@ void
ff_register_dynamic_payload_handler(RTPDynamicProtocolHandler *handler)
void ff_register_rtp_dynamic_payload_handlers(void)
{
+ff_register_dynamic_payload_handler(&ff_ac3_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_amr_nb_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_amr_wb_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_g726_16_dynamic_handler);
diff --git a/libavformat/rtpdec_ac3.c b/libavformat/rtpdec_ac3.c
new file mode 100644
index 000..2a47b75
--- /dev/null
+++ b/libavformat/rtpdec_ac3.c
@@ -0,0 +1,166 @@
+/*
+ * RTP parser for AC3 payload format
+ * Copyright (c) 2015 Gilles Chanteperdrix
+ *
+ * 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
+ */
+
+#include "avformat.h"
+#include "rtpdec_formats.h"
+#include "libavcodec/get_bits.h"
+
+/*
+ * From:
+ * https://tools.ietf.org/html/draft-ietf-avt-rtp-ac3-07
+ */
+
+struct PayloadContext {
+unsigned nr_frames;
+unsigned last_frame;
+uint32_t timestamp;
+AVIOContext *fragment;
+};
+
+static av_cold int
+ac3_init(AVFormatContext *s, int st_index, PayloadContext *data)
+{
+if (st_index < 0)
+return 0;
+s->streams[st_index]->need_parsing = AVSTREAM_PARSE_FULL;
+return 0;
+}
+
+static PayloadContext *ac3_new_context(void)
+{
+return av_mallocz(sizeof(PayloadContext));
+}
+
+static inline void free_fragment_if_needed(PayloadContext *data)
+{
+if (data->fragment) {
+uint8_t *p;
+avio_close_dyn_buf(data->fragment, &p);
+av_free(p);
+data->fragment = NULL;
+}
+}
+
+static void ac3_free_context(PayloadContext *data)
+{
+free_fragment_if_needed(data);
+av_free(data);
+}
+
+static int ac3_handle_packet(AVFormatContext *ctx, PayloadContext *data,
+ AVStream *st, AVPacket *pkt, uint32_t *timestamp,
+ const uint8_t *buf, int len, uint16_t seq,
+ int flags)
+{
+unsigned frame_type;
+unsigned nr_frames;
+int err;
+
+if (len < 2) {
+av_log(ctx, AV_LOG_ERROR, "Invalid %d bytes packet\n", len);
+return AVERROR_INVALIDDATA;
+}
+
+frame_type = buf[0] & 0x3;
+nr_frames = buf[1];
+buf += 2;
+len -= 2;
+
+switch (frame_type) {
+case 0: /* One or more complete frames */
+if (nr_frames > 1) {
+av_log(ctx, AV_LOG_ERROR,
+ "Unimplemented multiple AC3 frames per packet\n");
+return AVERROR_PATCHWELCOME;
+}
+if (nr_frames == 0) {
+av_log(ctx, AV_LOG_ERROR, "Invalid AC3 packet data\n");
+return AVERROR_INVALIDDATA;
+}
+if (av_new_packet(pkt, len)) {
+av_log(ctx, AV_LOG_ERROR, "Out of memory.\n");
+return AVERROR(ENOMEM);
+}
+
+pkt->stream_index = st->index;
+memcpy(pkt->data, buf, len);
+return 0;
+
+case 1:
+case 2: /*
[FFmpeg-devel] [PATCH 7/7] avformat/rtsp: parse lang attribute in SDP
---
libavformat/rtsp.c | 12
libavformat/rtsp.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 77407dd..3a68020 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -477,6 +477,8 @@ static void sdp_parse_line(AVFormatContext *s,
SDPParseState *s1,
handler->init(s, st->index,
rtsp_st->dynamic_protocol_context);
}
+if (rt->default_lang[0])
+av_dict_set(&st->metadata, "language", rt->default_lang, 0);
}
/* put a default control url */
av_strlcpy(rtsp_st->control_url, rt->control_uri,
@@ -540,6 +542,16 @@ static void sdp_parse_line(AVFormatContext *s,
SDPParseState *s1,
/* AV_NOPTS_VALUE means live broadcast (and can't seek) */
s->duration = (end == AV_NOPTS_VALUE) ?
AV_NOPTS_VALUE : end - start;
+} else if (av_strstart(p, "lang:", &p)) {
+if (s->nb_streams > 0) {
+get_word(buf1, sizeof(buf1), &p);
+rtsp_st = rt->rtsp_streams[rt->nb_rtsp_streams - 1];
+if (rtsp_st->stream_index >= 0) {
+st = s->streams[rtsp_st->stream_index];
+av_dict_set(&st->metadata, "language", buf1, 0);
+}
+} else
+get_word(rt->default_lang, sizeof(rt->default_lang), &p);
} else if (av_strstart(p, "IsRealDataType:integer;",&p)) {
if (atoi(p) == 1)
rt->transport = RTSP_TRANSPORT_RDT;
diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h
index a9f2f1e..3cfcfb6 100644
--- a/libavformat/rtsp.h
+++ b/libavformat/rtsp.h
@@ -405,6 +405,8 @@ typedef struct RTSPState {
* User-Agent string
*/
char *user_agent;
+
+char default_lang[4];
} RTSPState;
#define RTSP_FLAG_FILTER_SRC 0x1/**< Filter incoming UDP packets -
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/7] avcocdec/mpegaudio_parser: add MP3 ADU headers parser
---
libavcodec/mpegaudio_parser.c | 14 --
libavcodec/mpegaudiodecheader.c | 3 ++-
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/libavcodec/mpegaudio_parser.c b/libavcodec/mpegaudio_parser.c
index 79dbf63..42f4706 100644
--- a/libavcodec/mpegaudio_parser.c
+++ b/libavcodec/mpegaudio_parser.c
@@ -64,7 +64,7 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
}else{
while(icodec_id;
state= (state<<8) + buf[i++];
@@ -90,6 +90,16 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
avctx->bit_rate += (bit_rate - avctx->bit_rate) /
(s->header_count - header_threshold);
}
}
+
+if (s1->flags & PARSER_FLAG_COMPLETE_FRAMES) {
+s->frame_size = 0;
+next = buf_size;
+} else if (codec_id == AV_CODEC_ID_MP3ADU) {
+av_log(avctx, AV_LOG_ERROR,
+"MP3ADU full parser not implemented");
+return AVERROR_PATCHWELCOME;
+}
+
break;
}
}
@@ -110,7 +120,7 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
AVCodecParser ff_mpegaudio_parser = {
-.codec_ids = { AV_CODEC_ID_MP1, AV_CODEC_ID_MP2, AV_CODEC_ID_MP3 },
+.codec_ids = { AV_CODEC_ID_MP1, AV_CODEC_ID_MP2, AV_CODEC_ID_MP3,
AV_CODEC_ID_MP3ADU },
.priv_data_size = sizeof(MpegAudioParseContext),
.parser_parse = mpegaudio_parse,
.parser_close = ff_parse_close,
diff --git a/libavcodec/mpegaudiodecheader.c b/libavcodec/mpegaudiodecheader.c
index 5db1957..6af6e4b 100644
--- a/libavcodec/mpegaudiodecheader.c
+++ b/libavcodec/mpegaudiodecheader.c
@@ -134,7 +134,8 @@ int avpriv_mpa_decode_header2(uint32_t head, int
*sample_rate, int *channels, in
break;
default:
case 3:
-*codec_id = AV_CODEC_ID_MP3;
+if (*codec_id != AV_CODEC_ID_MP3ADU)
+*codec_id = AV_CODEC_ID_MP3;
if (s->lsf)
*frame_size = 576;
else
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/7] avformat/rtpdec: add T.140 RTP depacketization (RFC 4103)
---
libavformat/rtpdec.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index f5557d8..4091bdc 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -56,6 +56,12 @@ static RTPDynamicProtocolHandler opus_dynamic_handler = {
.codec_id = AV_CODEC_ID_OPUS,
};
+static RTPDynamicProtocolHandler ff_t140_dynamic_handler = {
+.enc_name = "t140",
+.codec_type = AVMEDIA_TYPE_SUBTITLE,
+.codec_id = AV_CODEC_ID_SUBRIP,
+};
+
static RTPDynamicProtocolHandler *rtp_first_dynamic_payload_handler = NULL;
void ff_register_dynamic_payload_handler(RTPDynamicProtocolHandler *handler)
@@ -96,6 +102,7 @@ void ff_register_rtp_dynamic_payload_handlers(void)
ff_register_dynamic_payload_handler(&ff_quicktime_rtp_aud_handler);
ff_register_dynamic_payload_handler(&ff_quicktime_rtp_vid_handler);
ff_register_dynamic_payload_handler(&ff_svq3_dynamic_handler);
+ff_register_dynamic_payload_handler(&ff_t140_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_theora_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_vorbis_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_vp8_dynamic_handler);
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 6/7] avformat/rtpdec_mpeg4: reassemble fragmented AAC frames
---
libavformat/rtpdec_mpeg4.c | 84 +-
1 file changed, 76 insertions(+), 8 deletions(-)
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 53921f2..f791a5c 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -33,6 +33,8 @@
#include "libavutil/avstring.h"
#include "libavcodec/get_bits.h"
+#define MAX_AAC_HBR_FRAME_SIZE 8191
+
/** Structure listing useful vars to parse RTP packet payload */
struct PayloadContext {
int sizelength;
@@ -59,8 +61,9 @@ struct PayloadContext {
int au_headers_length_bytes;
int cur_au_index;
-uint8_t buf[RTP_MAX_PACKET_LENGTH];
+uint8_t buf[FFMAX(RTP_MAX_PACKET_LENGTH, MAX_AAC_HBR_FRAME_SIZE)];
int buf_pos, buf_size;
+uint32_t timestamp;
};
typedef struct {
@@ -168,30 +171,95 @@ static int aac_parse_packet(AVFormatContext *ctx,
PayloadContext *data,
{
int ret;
+
if (!buf) {
-if (data->cur_au_index > data->nb_au_headers)
+if (data->cur_au_index > data->nb_au_headers) {
+av_log(ctx, AV_LOG_ERROR, "Invalid parser state\n");
return AVERROR_INVALIDDATA;
-if (data->buf_size - data->buf_pos <
data->au_headers[data->cur_au_index].size)
+}
+if (data->buf_size - data->buf_pos <
data->au_headers[data->cur_au_index].size) {
+av_log(ctx, AV_LOG_ERROR, "Invalid AU size\n");
return AVERROR_INVALIDDATA;
-if ((ret = av_new_packet(pkt,
data->au_headers[data->cur_au_index].size)) < 0)
+}
+if ((ret = av_new_packet(pkt,
data->au_headers[data->cur_au_index].size)) < 0) {
+av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
return ret;
+}
memcpy(pkt->data, &data->buf[data->buf_pos],
data->au_headers[data->cur_au_index].size);
data->buf_pos += data->au_headers[data->cur_au_index].size;
pkt->stream_index = st->index;
data->cur_au_index++;
-return data->cur_au_index < data->nb_au_headers;
+
+if (data->cur_au_index == data->nb_au_headers) {
+data->buf_pos = 0;
+return 0;
+}
+
+return 1;
}
-if (rtp_parse_mp4_au(data, buf, len))
+if (rtp_parse_mp4_au(data, buf, len)) {
+av_log(ctx, AV_LOG_ERROR, "Error parsing AU headers\n");
return -1;
+}
buf += data->au_headers_length_bytes + 2;
len -= data->au_headers_length_bytes + 2;
+if (data->nb_au_headers == 1 && len < data->au_headers[0].size) {
+/* Packet is fragmented */
+
+if (data->buf_pos == 0) {
+if (data->au_headers[0].size > MAX_AAC_HBR_FRAME_SIZE) {
+av_log(ctx, AV_LOG_ERROR, "Invalid AU size\n");
+return AVERROR_INVALIDDATA;
+}
+
+data->buf_size = data->au_headers[0].size;
+data->timestamp = *timestamp;
+}
+
+if (data->timestamp != *timestamp
+|| data->au_headers[0].size != data->buf_size
+|| data->buf_pos + len > MAX_AAC_HBR_FRAME_SIZE) {
+data->buf_pos = 0;
+data->buf_size = 0;
+av_log(ctx, AV_LOG_ERROR, "Invalid packet received\n");
+return AVERROR_INVALIDDATA;
+}
-if (len < data->au_headers[0].size)
+memcpy(&data->buf[data->buf_pos], buf, len);
+data->buf_pos += len;
+
+if (!(flags & RTP_FLAG_MARKER))
+return AVERROR(EAGAIN);
+
+if (data->buf_pos != data->buf_size) {
+data->buf_pos = 0;
+av_log(ctx, AV_LOG_ERROR, "Missed some packets, discarding
frame\n");
+return AVERROR_INVALIDDATA;
+}
+
+data->buf_pos = 0;
+ret = av_new_packet(pkt, data->buf_size);
+if (ret < 0) {
+av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
+return ret;
+}
+pkt->stream_index = st->index;
+
+memcpy(pkt->data, data->buf, data->buf_size);
+
+return 0;
+}
+
+if (len < data->au_headers[0].size) {
+av_log(ctx, AV_LOG_ERROR, "First AU larger than packet size\n");
return AVERROR_INVALIDDATA;
-if ((ret = av_new_packet(pkt, data->au_headers[0].size)) < 0)
+}
+if ((ret = av_new_packet(pkt, data->au_headers[0].size)) < 0) {
+av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
return ret;
+}
memcpy(pkt->data, buf, data->au_headers[0].size);
len -= data->au_headers[0].size;
buf += data->au_headers[0].size;
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 5/7] avformat/rtpdec_mpeg12: add robust MPEG audio depacketization (RFC 3119)
---
libavformat/rtpdec.c | 1 +
libavformat/rtpdec_formats.h | 1 +
libavformat/rtpdec_mpeg12.c | 201 +++
3 files changed, 203 insertions(+)
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index 4091bdc..711a94d 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -90,6 +90,7 @@ void ff_register_rtp_dynamic_payload_handlers(void)
ff_register_dynamic_payload_handler(&ff_mp4a_latm_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_mp4v_es_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_mpeg_audio_dynamic_handler);
+ff_register_dynamic_payload_handler(&ff_mpeg_audio_robust_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_mpeg_video_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_mpeg4_generic_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_mpegts_dynamic_handler);
diff --git a/libavformat/rtpdec_formats.h b/libavformat/rtpdec_formats.h
index e5aff59..3cbd0aa 100644
--- a/libavformat/rtpdec_formats.h
+++ b/libavformat/rtpdec_formats.h
@@ -57,6 +57,7 @@ extern RTPDynamicProtocolHandler ff_jpeg_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mp4a_latm_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mp4v_es_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mpeg_audio_dynamic_handler;
+extern RTPDynamicProtocolHandler ff_mpeg_audio_robust_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mpeg_video_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mpeg4_generic_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mpegts_dynamic_handler;
diff --git a/libavformat/rtpdec_mpeg12.c b/libavformat/rtpdec_mpeg12.c
index d73ff1e..7d01ef8 100644
--- a/libavformat/rtpdec_mpeg12.c
+++ b/libavformat/rtpdec_mpeg12.c
@@ -71,3 +71,204 @@ RTPDynamicProtocolHandler ff_mpeg_video_dynamic_handler = {
.parse_packet = mpeg_parse_packet,
.static_payload_id = 32,
};
+
+/* MPA-ROBUST, RFC 3119 */
+struct PayloadContext {
+unsigned adu_size;
+unsigned cur_size;
+uint32_t timestamp;
+uint8_t *split_buf;
+int split_pos, split_buf_size, split_pkts;
+AVIOContext *fragment;
+};
+
+static av_cold int
+mpa_robust_init(AVFormatContext *ctx, int st_index, PayloadContext *data)
+{
+if (st_index < 0)
+return 0;
+ctx->streams[st_index]->need_parsing = AVSTREAM_PARSE_HEADERS;
+return 0;
+}
+
+static PayloadContext *mpa_robust_new_context(void)
+{
+return av_mallocz(sizeof(PayloadContext));
+}
+
+static inline void free_fragment_if_needed(PayloadContext *data)
+{
+if (data->fragment) {
+uint8_t *p;
+avio_close_dyn_buf(data->fragment, &p);
+av_free(p);
+data->fragment = NULL;
+}
+}
+
+static void mpa_robust_free_context(PayloadContext *data)
+{
+free_fragment_if_needed(data);
+av_free(data);
+}
+
+static int mpa_robust_parse_rtp_header(AVFormatContext *ctx,
+const uint8_t *buf, int len,
+unsigned *adu_size, unsigned *cont)
+{
+unsigned header_size;
+
+if (len < 1) {
+av_log(ctx, AV_LOG_ERROR, "Invalid %d bytes packet\n", len);
+return AVERROR_INVALIDDATA;
+}
+
+*cont = !!(buf[0] & 0x80);
+if ((buf[0] & 0x40) == 0) {
+header_size = 1;
+*adu_size = buf[0] & ~0xc0;
+} else {
+if (len == 1) {
+av_log(ctx, AV_LOG_ERROR, "Invalid %d bytes packet\n", len);
+return AVERROR_INVALIDDATA;
+}
+header_size = 2;
+*adu_size = AV_RB16(buf) & ~0xc000;
+}
+
+return header_size;
+}
+
+static int
+mpa_robust_parse_packet(AVFormatContext *ctx, PayloadContext *data,
+AVStream *st, AVPacket *pkt, uint32_t *timestamp,
+const uint8_t *buf, int len, uint16_t seq,
+int flags)
+{
+unsigned adu_size, continuation;
+int err;
+
+if (!buf) {
+buf = &data->split_buf[data->split_pos];
+len = data->split_buf_size - data->split_pos;
+
+err = mpa_robust_parse_rtp_header(ctx, buf, len, &adu_size,
&continuation);
+if (err < 0) {
+av_freep(&data->split_buf);
+return err;
+}
+buf += err;
+len -= err;
+
+if (continuation != 0 || adu_size > len) {
+av_freep(&data->split_buf);
+av_log(ctx, AV_LOG_ERROR, "Invalid frame\n");
+return AVERROR_INVALIDDATA;
+}
+
+if (av_new_packet(pkt, adu_size)) {
+av_log(ctx, AV_LOG_ERROR, "Out of memory.\n");
+return AVERROR(ENOMEM);
+}
+
+pkt->stream_index = st->index;
+memcpy(pkt->data, buf, adu_size);
+
+data->split_pos = (buf - data->split_buf) + adu_size;
+
+if (data->split_pos == data->split_buf_size) {
+av_freep(&data->split_buf);
+return 0;
+}
+
+re
Re: [FFmpeg-devel] [PATCH 0/7] using ffmpeg as an RTSP client
On Sun, Feb 08, 2015 at 10:20:20PM +0100, Gilles Chanteperdrix wrote:
> Hi,
>
> I am using ffmpeg 2.5.3 as an RTSP client and found a few missing
> pieces, which the following series of patches should address.
Hi,
ping ?
I sent these patches almost a week ago and received no answer. Is it
because the patches are not conform to the submission rules, or
simply because people are to busy for reviewing them?
Thanks in advance.
Regards.
--
Gilles.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 0/7] using ffmpeg as an RTSP client
On Fri, Feb 13, 2015 at 07:45:27PM +0100, Gilles Chanteperdrix wrote: > On Sun, Feb 08, 2015 at 10:20:20PM +0100, Gilles Chanteperdrix wrote: > > Hi, > > > > I am using ffmpeg 2.5.3 as an RTSP client and found a few missing > > pieces, which the following series of patches should address. > > Hi, > > ping ? > > I sent these patches almost a week ago and received no answer. Is it > because the patches are not conform to the submission rules, or > simply because people are to busy for reviewing them? Never mind. Sorry for the noise. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/7] avformat/rtpdec_ac3: add AC3 RTP depacketization
On Thu, Feb 12, 2015 at 10:07:39PM +0100, Thomas Volkert wrote:
> >+if (nr_frames > 1) {
> >+av_log(ctx, AV_LOG_ERROR,
> >+ "Unimplemented multiple AC3 frames per packet\n");
>
> You could use avpriv_report_missing_feature() here.
I am not sure that the case where there are multiple frames in one
packet is not already working: frames are concatenated without not
headers, so the full AC3 parser should be able to split the packet
in multiple frames right ? Unfortunately, I do not have an example
using this feature.
> >+if (nr_frames > 1) {
> >+av_log(ctx, AV_LOG_ERROR,
> >+ "Unimplemented multiple AC3 frames per packet\n");
>
> You could use avpriv_report_missing_feature() here.
I am not sure that the case where there are multiple frames in one
packet is not already working: frames are concatenated without not
headers, so the full AC3 parser should be able to split the packet
in multiple frames right ? Unfortunately, I do not have an example
using this feature.
>
> Do you see any use for the optional SDP parameter?
> If yes, you could add an AC3 specific parser for SDP line and link it as
> ".parse_sdp_a_line" line here.
Which optional SDP parameter? channels,ptime,maxptime ?
> What do you think about a packetizer as counterpart?
Currently, I am using live555 as an RTSP server. I did not check
ffmpeg documentation to see how to run an RTSP server. I will check.
--
Gilles.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] RTP patches v2
Hi, please find a second version of some patches for various RTP audio payload format. This second version addresses Thomas Volkert comments. Regards. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] avformat/rtpdec_mpeg4: reassemble fragmented AAC frames
Signed-off-by: Gilles Chanteperdrix
---
libavformat/rtpdec_mpeg4.c | 84 +-
1 file changed, 76 insertions(+), 8 deletions(-)
diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 9655d30..f2814b5 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -33,6 +33,8 @@
#include "libavutil/avstring.h"
#include "libavcodec/get_bits.h"
+#define MAX_AAC_HBR_FRAME_SIZE 8191
+
/** Structure listing useful vars to parse RTP packet payload */
struct PayloadContext {
int sizelength;
@@ -59,8 +61,9 @@ struct PayloadContext {
int au_headers_length_bytes;
int cur_au_index;
-uint8_t buf[RTP_MAX_PACKET_LENGTH];
+uint8_t buf[FFMAX(RTP_MAX_PACKET_LENGTH, MAX_AAC_HBR_FRAME_SIZE)];
int buf_pos, buf_size;
+uint32_t timestamp;
};
typedef struct {
@@ -168,30 +171,95 @@ static int aac_parse_packet(AVFormatContext *ctx,
PayloadContext *data,
{
int ret;
+
if (!buf) {
-if (data->cur_au_index > data->nb_au_headers)
+if (data->cur_au_index > data->nb_au_headers) {
+av_log(ctx, AV_LOG_ERROR, "Invalid parser state\n");
return AVERROR_INVALIDDATA;
-if (data->buf_size - data->buf_pos <
data->au_headers[data->cur_au_index].size)
+}
+if (data->buf_size - data->buf_pos <
data->au_headers[data->cur_au_index].size) {
+av_log(ctx, AV_LOG_ERROR, "Invalid AU size\n");
return AVERROR_INVALIDDATA;
-if ((ret = av_new_packet(pkt,
data->au_headers[data->cur_au_index].size)) < 0)
+}
+if ((ret = av_new_packet(pkt,
data->au_headers[data->cur_au_index].size)) < 0) {
+av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
return ret;
+}
memcpy(pkt->data, &data->buf[data->buf_pos],
data->au_headers[data->cur_au_index].size);
data->buf_pos += data->au_headers[data->cur_au_index].size;
pkt->stream_index = st->index;
data->cur_au_index++;
-return data->cur_au_index < data->nb_au_headers;
+
+if (data->cur_au_index == data->nb_au_headers) {
+data->buf_pos = 0;
+return 0;
+}
+
+return 1;
}
-if (rtp_parse_mp4_au(data, buf, len))
+if (rtp_parse_mp4_au(data, buf, len)) {
+av_log(ctx, AV_LOG_ERROR, "Error parsing AU headers\n");
return -1;
+}
buf += data->au_headers_length_bytes + 2;
len -= data->au_headers_length_bytes + 2;
+if (data->nb_au_headers == 1 && len < data->au_headers[0].size) {
+/* Packet is fragmented */
+
+if (!data->buf_pos) {
+if (data->au_headers[0].size > MAX_AAC_HBR_FRAME_SIZE) {
+av_log(ctx, AV_LOG_ERROR, "Invalid AU size\n");
+return AVERROR_INVALIDDATA;
+}
+
+data->buf_size = data->au_headers[0].size;
+data->timestamp = *timestamp;
+}
+
+if (data->timestamp != *timestamp
+|| data->au_headers[0].size != data->buf_size
+|| data->buf_pos + len > MAX_AAC_HBR_FRAME_SIZE) {
+data->buf_pos = 0;
+data->buf_size = 0;
+av_log(ctx, AV_LOG_ERROR, "Invalid packet received\n");
+return AVERROR_INVALIDDATA;
+}
-if (len < data->au_headers[0].size)
+memcpy(&data->buf[data->buf_pos], buf, len);
+data->buf_pos += len;
+
+if (!(flags & RTP_FLAG_MARKER))
+return AVERROR(EAGAIN);
+
+if (data->buf_pos != data->buf_size) {
+data->buf_pos = 0;
+av_log(ctx, AV_LOG_ERROR, "Missed some packets, discarding
frame\n");
+return AVERROR_INVALIDDATA;
+}
+
+data->buf_pos = 0;
+ret = av_new_packet(pkt, data->buf_size);
+if (ret < 0) {
+av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
+return ret;
+}
+pkt->stream_index = st->index;
+
+memcpy(pkt->data, data->buf, data->buf_size);
+
+return 0;
+}
+
+if (len < data->au_headers[0].size) {
+av_log(ctx, AV_LOG_ERROR, "First AU larger than packet size\n");
return AVERROR_INVALIDDATA;
-if ((ret = av_new_packet(pkt, data->au_headers[0].size)) < 0)
+}
+if ((ret = av_new_packet(pkt, data->au_headers[0].size)) < 0) {
+av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
return ret;
+}
memcpy(pkt->data, buf, data->au_headers[0].size);
len -= data->au_headers[0].size;
buf += data->au_headers[0].size;
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] avformat/rtpdec_ac3: add AC3 RTP depacketization (RFC 4184)
Signed-off-by: Gilles Chanteperdrix
---
Changelog| 1 +
MAINTAINERS | 1 +
libavformat/Makefile | 1 +
libavformat/rtpdec.c | 1 +
libavformat/rtpdec_ac3.c | 157 +++
libavformat/rtpdec_formats.h | 1 +
libavformat/version.h| 4 +-
7 files changed, 164 insertions(+), 2 deletions(-)
create mode 100644 libavformat/rtpdec_ac3.c
diff --git a/Changelog b/Changelog
index 775fc79..085fd17 100644
--- a/Changelog
+++ b/Changelog
@@ -24,6 +24,7 @@ version :
- ported softpulldown filter from libmpcodecs as repeatfields filter
- dcshift filter
- RTP parser for loss tolerant payload format for MP3 audio (RFC 5219)
+- RTP parser for AC3 payload format (RFC 4184)
version 2.5:
diff --git a/MAINTAINERS b/MAINTAINERS
index 13b211e..8430222 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -465,6 +465,7 @@ Muxers/Demuxers:
rmdec.c, rmenc.c Ronald S. Bultje, Kostya Shishkov
rtmp* Kostya Shishkov
rtp.c, rtpenc.c Martin Storsjo
+ rtpdec_ac3.* Gilles Chanteperdrix
rtpdec_h261.*, rtpenc_h261.* Thomas Volkert
rtpdec_hevc.*, rtpenc_hevc.* Thomas Volkert
rtpdec_asf.* Ronald S. Bultje
diff --git a/libavformat/Makefile b/libavformat/Makefile
index ec312ba..073a42d 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -29,6 +29,7 @@ OBJS-$(CONFIG_RIFFENC) += riffenc.o
OBJS-$(CONFIG_RTPDEC)+= rdt.o \
rtp.o \
rtpdec.o\
+rtpdec_ac3.o\
rtpdec_amr.o\
rtpdec_asf.o\
rtpdec_g726.o \
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index c632340..550da4a 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -72,6 +72,7 @@ void
ff_register_dynamic_payload_handler(RTPDynamicProtocolHandler *handler)
void ff_register_rtp_dynamic_payload_handlers(void)
{
+ff_register_dynamic_payload_handler(&ff_ac3_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_amr_nb_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_amr_wb_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_g726_16_dynamic_handler);
diff --git a/libavformat/rtpdec_ac3.c b/libavformat/rtpdec_ac3.c
new file mode 100644
index 000..728f421
--- /dev/null
+++ b/libavformat/rtpdec_ac3.c
@@ -0,0 +1,157 @@
+/*
+ * RTP parser for AC3 payload format (RFC 4184)
+ * Copyright (c) 2015 Gilles Chanteperdrix
+ *
+ * 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
+ */
+
+#include "avformat.h"
+#include "rtpdec_formats.h"
+
+#define RTP_AC3_PAYLOAD_HEADER_SIZE 2
+
+struct PayloadContext {
+unsigned nr_frames;
+unsigned last_frame;
+uint32_t timestamp;
+AVIOContext *fragment;
+};
+
+static av_cold int ac3_init(AVFormatContext *s, int st_index,
+ PayloadContext *data)
+{
+if (st_index < 0)
+return 0;
+s->streams[st_index]->need_parsing = AVSTREAM_PARSE_FULL;
+return 0;
+}
+
+static PayloadContext *ac3_new_context(void)
+{
+return av_mallocz(sizeof(PayloadContext));
+}
+
+static inline void free_fragment_if_needed(PayloadContext *data)
+{
+if (data->fragment) {
+uint8_t *p;
+avio_close_dyn_buf(data->fragment, &p);
+av_free(p);
+data->fragment = NULL;
+}
+}
+
+static void ac3_free_context(PayloadContext *data)
+{
+free_fragment_if_needed(data);
+av_free(data);
+}
+
+static int ac3_handle_packet(AVFormatContext *ctx, PayloadContext *data,
+ AVStream *st, AVPacket *pkt, uint32_t *timestamp,
+ const uint8_t *buf, int len, uint16_t seq,
+ int flags)
+{
+unsigned frame_t
[FFmpeg-devel] [PATCH 2/3] avformat/rtpdec_mpeg12: add robust MPEG audio depacketization (RFC 5219)
Signed-off-by: Gilles Chanteperdrix
---
Changelog | 1 +
libavcodec/mpegaudio_parser.c | 14 ++-
libavcodec/mpegaudiodecheader.c | 3 +-
libavformat/rtpdec.c| 1 +
libavformat/rtpdec_formats.h| 1 +
libavformat/rtpdec_mpeg12.c | 204
libavformat/version.h | 4 +-
7 files changed, 223 insertions(+), 5 deletions(-)
diff --git a/Changelog b/Changelog
index c663d5e..775fc79 100644
--- a/Changelog
+++ b/Changelog
@@ -23,6 +23,7 @@ version :
- Changed default DNxHD colour range in QuickTime .mov derivatives to mpeg
range
- ported softpulldown filter from libmpcodecs as repeatfields filter
- dcshift filter
+- RTP parser for loss tolerant payload format for MP3 audio (RFC 5219)
version 2.5:
diff --git a/libavcodec/mpegaudio_parser.c b/libavcodec/mpegaudio_parser.c
index 79dbf63..42f4706 100644
--- a/libavcodec/mpegaudio_parser.c
+++ b/libavcodec/mpegaudio_parser.c
@@ -64,7 +64,7 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
}else{
while(icodec_id;
state= (state<<8) + buf[i++];
@@ -90,6 +90,16 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
avctx->bit_rate += (bit_rate - avctx->bit_rate) /
(s->header_count - header_threshold);
}
}
+
+if (s1->flags & PARSER_FLAG_COMPLETE_FRAMES) {
+s->frame_size = 0;
+next = buf_size;
+} else if (codec_id == AV_CODEC_ID_MP3ADU) {
+av_log(avctx, AV_LOG_ERROR,
+"MP3ADU full parser not implemented");
+return AVERROR_PATCHWELCOME;
+}
+
break;
}
}
@@ -110,7 +120,7 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
AVCodecParser ff_mpegaudio_parser = {
-.codec_ids = { AV_CODEC_ID_MP1, AV_CODEC_ID_MP2, AV_CODEC_ID_MP3 },
+.codec_ids = { AV_CODEC_ID_MP1, AV_CODEC_ID_MP2, AV_CODEC_ID_MP3,
AV_CODEC_ID_MP3ADU },
.priv_data_size = sizeof(MpegAudioParseContext),
.parser_parse = mpegaudio_parse,
.parser_close = ff_parse_close,
diff --git a/libavcodec/mpegaudiodecheader.c b/libavcodec/mpegaudiodecheader.c
index 5db1957..6af6e4b 100644
--- a/libavcodec/mpegaudiodecheader.c
+++ b/libavcodec/mpegaudiodecheader.c
@@ -134,7 +134,8 @@ int avpriv_mpa_decode_header2(uint32_t head, int
*sample_rate, int *channels, in
break;
default:
case 3:
-*codec_id = AV_CODEC_ID_MP3;
+if (*codec_id != AV_CODEC_ID_MP3ADU)
+*codec_id = AV_CODEC_ID_MP3;
if (s->lsf)
*frame_size = 576;
else
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index 4ff209c..c632340 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -89,6 +89,7 @@ void ff_register_rtp_dynamic_payload_handlers(void)
ff_register_dynamic_payload_handler(&ff_mp4a_latm_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_mp4v_es_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_mpeg_audio_dynamic_handler);
+ff_register_dynamic_payload_handler(&ff_mpeg_audio_robust_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_mpeg_video_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_mpeg4_generic_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_mpegts_dynamic_handler);
diff --git a/libavformat/rtpdec_formats.h b/libavformat/rtpdec_formats.h
index 87e316f..da22d70 100644
--- a/libavformat/rtpdec_formats.h
+++ b/libavformat/rtpdec_formats.h
@@ -52,6 +52,7 @@ extern RTPDynamicProtocolHandler ff_jpeg_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mp4a_latm_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mp4v_es_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mpeg_audio_dynamic_handler;
+extern RTPDynamicProtocolHandler ff_mpeg_audio_robust_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mpeg_video_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mpeg4_generic_dynamic_handler;
extern RTPDynamicProtocolHandler ff_mpegts_dynamic_handler;
diff --git a/libavformat/rtpdec_mpeg12.c b/libavformat/rtpdec_mpeg12.c
index d73ff1e..d99c507 100644
--- a/libavformat/rtpdec_mpeg12.c
+++ b/libavformat/rtpdec_mpeg12.c
@@ -2,6 +2,9 @@
* Common code for the RTP depacketization of MPEG-1/2 formats.
* Copyright (c) 2002 Fabrice Bellard
*
+ * RTP parser for loss tolerant payload format for MP3 audio (RFC 5219)
+ * Copyright (c) 2015 Gilles Chanteperdrix
+ *
* This file is part of FFmpeg.
*
* FFmpeg is free software; you can redistribute it and/or
@@ -23,6 +26,8 @@
#include "libavutil/intreadwrite.h"
#include "rtpdec_formats.h"
+#define RTP_MPA_PAYLOAD_HEADER_SIZE 1
+
Re: [FFmpeg-devel] [PATCH 2/3] avformat/rtpdec_mpeg12: add robust MPEG audio depacketization (RFC 5219)
On Sat, Feb 14, 2015 at 05:56:49PM +0100, Thomas Volkert wrote: > On 02/13/2015 10:51 PM, Gilles Chanteperdrix wrote: > > #define LIBAVFORMAT_VERSION_MAJOR 56 > >-#define LIBAVFORMAT_VERSION_MINOR 19 > >-#define LIBAVFORMAT_VERSION_MICRO 101 > >+#define LIBAVFORMAT_VERSION_MINOR 20 > >+#define LIBAVFORMAT_VERSION_MICRO 102 > In this case it should be: > > +#define LIBAVFORMAT_VERSION_MINOR 20 > +#define LIBAVFORMAT_VERSION_MICRO 100 > > My comment was maybe a bit misleading. But I think this can be fixed when the > patch gets committed. Sorry, I was completely ignorant of how major/minor/micro version worked. I can post new versions of the patches if needed. Thanks for your patient review. Regards. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] avformat/rtpdec_mpeg4: reassemble fragmented AAC frames
On Fri, Feb 13, 2015 at 10:51:32PM +0100, Gilles Chanteperdrix wrote:
> Signed-off-by: Gilles Chanteperdrix
> ---
> libavformat/rtpdec_mpeg4.c | 84
> +-
> 1 file changed, 76 insertions(+), 8 deletions(-)
Hi, what about this patch?
Regards.
--
Gilles.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavformat/rtpdec: fix issue with conversion from unsigned to signed
When receiving an RTCP packet, the difference between the last RTCP timestamp and the base timestamp may be negative. As these timestamps are of the uint32_t type, the result becomes a large integer. Cast the difference to int32_t to avoid this issue. The result of this issue is very large start times for RTSP streams, and difficulty to restart correctly after a pause. Signed-off-by: Gilles Chanteperdrix --- libavformat/rtpdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c index 711a94d..5cbcfa3 100644 --- a/libavformat/rtpdec.c +++ b/libavformat/rtpdec.c @@ -159,7 +159,7 @@ static int rtcp_parse_packet(RTPDemuxContext *s, const unsigned char *buf, s->first_rtcp_ntp_time = s->last_rtcp_ntp_time; if (!s->base_timestamp) s->base_timestamp = s->last_rtcp_timestamp; -s->rtcp_ts_offset = s->last_rtcp_timestamp - s->base_timestamp; +s->rtcp_ts_offset = (int32_t)(s->last_rtcp_timestamp - s->base_timestamp); } break; -- 1.8.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] avformat/rtpdec_mpeg4: reassemble fragmented AAC frames
On Sat, Feb 21, 2015 at 04:54:12PM +0100, Thomas Volkert wrote:
> >+if (len < data->au_headers[0].size) {
> >+av_log(ctx, AV_LOG_ERROR, "First AU larger than packet size\n");
>
> Can this really happen?
> In the lines above you already check for the case that data is missing while
> you have already received the last packet of the frame.
The first test tests for
data->nb_au_headers == 1 && len < data->au_headers[0].size
We could still have:
data->nb_au_headers >= 2 && len < data->au_headers[0].size
Which is illegal per the RFC (a packet contains either a piece of a
fragmented frame, or one or several entire frames, but never both),
but some malformed packets could happen.
--
Gilles.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/rtsp: punch holes again after pause
When a client behind a NAT issues a pause command, and stay paused for a long time, the router may stop the RTP/RTCP port redirection. Resend the hole punching packets after each PLAY command to cause the router to restart the port redirection in that case. Signed-off-by: Gilles Chanteperdrix --- libavformat/rtspdec.c | 4 1 file changed, 4 insertions(+) diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c index fa7383c..bfbe52b 100644 --- a/libavformat/rtspdec.c +++ b/libavformat/rtspdec.c @@ -542,6 +542,10 @@ static int rtsp_read_play(AVFormatContext *s) AVStream *st = NULL; if (!rtpctx || rtsp_st->stream_index < 0) continue; +if (CONFIG_RTPDEC && +!(rt->server_type == RTSP_SERVER_WMS && i > 1) && s->iformat) +ff_rtp_send_punch_packets(rtsp_st->rtp_handle); + st = s->streams[rtsp_st->stream_index]; rtpctx->range_start_offset = av_rescale_q(reply->range_start, AV_TIME_BASE_Q, -- 1.8.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] avfilter/af_volume: fix precision=fixed and volume=0 case
When precision is fixed and volume is 0, filter_frame does not
perform any operation on the output buffer. This works if the
output buffer has been allocated and zeroed with ff_get_audio_buffer
but not if the input buffer is used as output buffer.
Fix this by not using the input buffer as output buffer if
precision is fixed and volume is 0.
Signed-off-by: Gilles Chanteperdrix
---
libavfilter/af_volume.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libavfilter/af_volume.c b/libavfilter/af_volume.c
index 4809ae7..8d49c65 100644
--- a/libavfilter/af_volume.c
+++ b/libavfilter/af_volume.c
@@ -402,7 +402,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf)
}
/* do volume scaling in-place if input buffer is writable */
-if (av_frame_is_writable(buf)) {
+if (av_frame_is_writable(buf)
+&& (vol->precision != PRECISION_FIXED || vol->volume_i > 0)) {
out_buf = buf;
} else {
out_buf = ff_get_audio_buffer(inlink, nb_samples);
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/3] avformat: map T.140 RTP codec to text
This makes more sense than mapping to AV_CODEC_ID_SUBRIP. Nothing
indicates that a T.140 track contains subrip sub-titles.
Signed-off-by: Gilles Chanteperdrix
---
libavformat/rtpdec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index 8cd2f26..2227e23 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -59,7 +59,7 @@ static RTPDynamicProtocolHandler opus_dynamic_handler = {
static RTPDynamicProtocolHandler t140_dynamic_handler = { /* RFC 4103 */
.enc_name = "t140",
.codec_type = AVMEDIA_TYPE_SUBTITLE,
-.codec_id = AV_CODEC_ID_SUBRIP,
+.codec_id = AV_CODEC_ID_TEXT,
};
static RTPDynamicProtocolHandler *rtp_first_dynamic_payload_handler = NULL;
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] Revert "rtsp: Interpret the text media type as AVMEDIA_TYPE_DATA"
This reverts commit 26524e358147aade6e9dd18fff42d61b966bbc70.
If we want the T.140 codec to have the AV_CODEC_ID_TEXT codec id,
its type needs to be AVMEDIA_TYPE_SUBTITLE, so, keep interpreting
the text media type as AVMEDIA_TYPE_SUBTITLE.
Signed-off-by: Gilles Chanteperdrix
---
libavformat/rtsp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 05f4171..91128e1 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -418,7 +418,7 @@ static void sdp_parse_line(AVFormatContext *s,
SDPParseState *s1,
codec_type = AVMEDIA_TYPE_AUDIO;
} else if (!strcmp(st_type, "video")) {
codec_type = AVMEDIA_TYPE_VIDEO;
-} else if (!strcmp(st_type, "application") || !strcmp(st_type,
"text")) {
+} else if (!strcmp(st_type, "application")) {
codec_type = AVMEDIA_TYPE_DATA;
} else if (!strcmp(st_type, "text")) {
codec_type = AVMEDIA_TYPE_SUBTITLE;
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] avformat: map T.140 RTP codec to text
On Sun, Mar 01, 2015 at 08:25:12AM +0100, Clément Bœsch wrote:
> On Sat, Feb 28, 2015 at 11:15:40PM +0100, Gilles Chanteperdrix wrote:
> > This makes more sense than mapping to AV_CODEC_ID_SUBRIP. Nothing
> > indicates that a T.140 track contains subrip sub-titles.
> >
> > Signed-off-by: Gilles Chanteperdrix
> > ---
> > libavformat/rtpdec.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
> > index 8cd2f26..2227e23 100644
> > --- a/libavformat/rtpdec.c
> > +++ b/libavformat/rtpdec.c
> > @@ -59,7 +59,7 @@ static RTPDynamicProtocolHandler opus_dynamic_handler = {
> > static RTPDynamicProtocolHandler t140_dynamic_handler = { /* RFC 4103 */
> > .enc_name = "t140",
> > .codec_type = AVMEDIA_TYPE_SUBTITLE,
> > -.codec_id = AV_CODEC_ID_SUBRIP,
> > +.codec_id = AV_CODEC_ID_TEXT,
> > };
> >
>
> What do the subs look like? Is it really just plaintext or there is some
> binary or whatever markup?
The T.140 RTP payload is for transmitting plain text. What really
gets transmitted depends on what the server decides to send. I
originally chose subrip, because that is the way vlc decodes these
tracks, but really, this is not standard.
--
Gilles.
pgpEMrkTRL7X1.pgp
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] avformat/rtpdec_mpa_robust: fix commit 96084251e57d1738fde02a2b0d37ca609d9efd71
Commit 96084251e57d1738fde02a2b0d37ca609d9efd71 introduced a change in
the parser implementation which broke it. Restore the original
implementation.
Signed-off-by: Gilles Chanteperdrix
---
libavformat/rtpdec_mpa_robust.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/rtpdec_mpa_robust.c b/libavformat/rtpdec_mpa_robust.c
index 776e245..2439f9e 100644
--- a/libavformat/rtpdec_mpa_robust.c
+++ b/libavformat/rtpdec_mpa_robust.c
@@ -98,7 +98,7 @@ static int mpa_robust_parse_packet(AVFormatContext *ctx,
PayloadContext *data,
pkt->stream_index = st->index;
memcpy(pkt->data, buf, adu_size);
-data->split_pos += adu_size;
+data->split_pos = (buf - data->split_buf) + adu_size;
if (data->split_pos == data->split_buf_size) {
av_freep(&data->split_buf);
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] MAINTAINERS: add myself as rtpdec_mpa_robust maintainer
Signed-off-by: Gilles Chanteperdrix --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 0cd8c26..007540c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -471,6 +471,7 @@ Muxers/Demuxers: rtpdec_dv.* Thomas Volkert rtpdec_h261.*, rtpenc_h261.* Thomas Volkert rtpdec_hevc.*, rtpenc_hevc.* Thomas Volkert + rtpdec_mpa_robust.* Gilles Chanteperdrix rtpdec_asf.* Ronald S. Bultje rtpdec_vp9.c Thomas Volkert rtpenc_mpv.*, rtpenc_aac.*Martin Storsjo -- 1.8.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
When transcoding to SRT, do not assume that each sub-title frame contains
text.
Signed-off-by: Gilles Chanteperdrix
---
libavcodec/srtenc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavcodec/srtenc.c b/libavcodec/srtenc.c
index 3287970..ab790f8 100644
--- a/libavcodec/srtenc.c
+++ b/libavcodec/srtenc.c
@@ -241,6 +241,9 @@ static int srt_encode_frame(AVCodecContext *avctx,
for (; dialog && num--; dialog++) {
s->alignment_applied = 0;
srt_style_apply(s, dialog->style);
+if (!dialog->text)
+continue;
+
ff_ass_split_override_codes(&srt_callbacks, s, dialog->text);
}
}
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
On Sat, Mar 07, 2015 at 09:18:41PM +0100, Nicolas George wrote:
> Le septidi 17 ventôse, an CCXXIII, Gilles Chanteperdrix a écrit :
> > When transcoding to SRT, do not assume that each sub-title frame contains
> > text.
>
> Can you explain how to reproduce the segfault? I tried transcoding an ASS
> file and it worked.
You need an ASS file that produces a sub-title frame without text. I
have been carrying this patch since 2013, I am afraid I no longer
have the problematic file.
--
Gilles.
pgprvrq1x6I6o.pgp
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
On Sat, Mar 07, 2015 at 11:46:44PM +0100, Nicolas George wrote: > Le septidi 17 ventôse, an CCXXIII, Gilles Chanteperdrix a écrit : > > You need an ASS file that produces a sub-title frame without text. I > > have been carrying this patch since 2013, I am afraid I no longer > > have the problematic file. > > I tried an ASS file with an empty text part, and it did not crash > either. Is not there some way in the ssa format to declare some graphic elements which are not text? > It > is entirely possible the problem has disappeared due to some of the reworks > of the text subtitles code since you experienced it. In my modest experience, bugs do not just disappear if you do not fix them. > > I am sorry to insist, but I am always suspicious of changes that go "if the > value is not what we expect, silently ignore it", because they frequently > should return an error or hide a bug earlier in the code; Maybe it is not a perfect patch, but if subtitles can contain graphic elements mixed with text, it seems better to translate the text only when the target codec only supports text, than to fail with a segmentation fault. > for these reasons, > I would like either a more detailed analysis or a chance to see for myself > before this patch is applied. Just to make things clear: you will not get such an analysis for me. I am perfectly fine with the patch applied in my local copy of ffmpeg. -- Gilles. pgpgGnMK76Xzl.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
On Sat, Mar 07, 2015 at 11:46:44PM +0100, Nicolas George wrote: > Le septidi 17 ventôse, an CCXXIII, Gilles Chanteperdrix a écrit : > > You need an ASS file that produces a sub-title frame without text. I > > have been carrying this patch since 2013, I am afraid I no longer > > have the problematic file. > > I tried an ASS file with an empty text part, and it did not crash either. It > is entirely possible the problem has disappeared due to some of the reworks > of the text subtitles code since you experienced it. > > I am sorry to insist, but I am always suspicious of changes that go "if the > value is not what we expect, silently ignore it", because they frequently > should return an error or hide a bug earlier in the code; for these reasons, > I would like either a more detailed analysis or a chance to see for myself > before this patch is applied. If I read ffmpeg current documentation here: http://www.ffmpeg.org/doxygen/2.5/group__lavc__core.html#ga828218dcb8874ab2c5af8d486c365421 It seems that when a subtitle rect type is SUBTITLE_ASS (which is the case in the code which my patch modified), there is no guarantee that the text field contains something. -- Gilles. pgpoVl06GLm91.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/srtenc: avoid segmentation fault when transcoding from SSA to SRT
On Sun, Mar 08, 2015 at 10:57:54AM +0100, Nicolas George wrote:
> L'octidi 18 ventôse, an CCXXIII, Clement Boesch a écrit :
> > Try adding tags with no text maybe. You may also try ASS drawing mode, but
> > FFmpeg probably doesn't do cray stuff about it.
>
> I tried various combinations, including empty styles, drawing mode, Picture,
> Comment, empty lines, missing comma, nothing produces a crash.
Ok, I will remove the patch on my side, and chime in if I get the
segfault again. Thanks for investigating, sorry for the noise.
Regards.
--
Gilles.
pgpfq8h5xrkbH.pgp
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Segmentation fault in libquvi.c
Hi,
I just triend libquvi, and get a segmentation fault in the
libquvi_read_header function, because ff_copy_whitelists is called
before qc->fmtctx is allocated by avformat_open_input. I added a
call to avformat_alloc_context() before ff_copy_whitelists and the
libquvi demuxer works.
However, I wonder how to fix this properly: the error handling
labels look backward, so that I am not sure where to free the
allocated context in case of error.
Regards.
--
Gilles.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Segmentation fault in libquvi.c
On Wed, Apr 08, 2015 at 11:02:47AM +0200, wm4 wrote: > On Wed, 8 Apr 2015 10:55:45 +0200 > Gilles Chanteperdrix wrote: > > > Hi, > > > > I just triend libquvi, and get a segmentation fault in the > > libquvi_read_header function, because ff_copy_whitelists is called > > before qc->fmtctx is allocated by avformat_open_input. I added a > > call to avformat_alloc_context() before ff_copy_whitelists and the > > libquvi demuxer works. > > > > However, I wonder how to fix this properly: the error handling > > labels look backward, so that I am not sure where to free the > > allocated context in case of error. > > > > Regards. > > > > It's probably best to remove libquvi. It's dead. Indeed, no activity since 2013. Is there any other ffmpeg demuxer providing the same functionality ? -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Segmentation fault in libquvi.c
On Wed, Apr 08, 2015 at 02:40:52PM +0200, Michael Niedermayer wrote: > On Wed, Apr 08, 2015 at 10:55:45AM +0200, Gilles Chanteperdrix wrote: > > Hi, > > > > I just triend libquvi, and get a segmentation fault in the > > libquvi_read_header function, because ff_copy_whitelists is called > > before qc->fmtctx is allocated by avformat_open_input. I added a > > call to avformat_alloc_context() before ff_copy_whitelists and the > > libquvi demuxer works. > > > > However, I wonder how to fix this properly: the error handling > > labels look backward, so that I am not sure where to free the > > allocated context in case of error. > > applied this, yes its correct Ok, there are other details missing, the stream does not get a duration, start_time and bitrate. This can easily be fixed, but as wm4 said libquvi seems an abandoned project. Would there be any interest in a solution based on youtube-dl? It seems to be the standard, these days. I just ran a few tests: youtube -qs 'url' returns 0 or 1 depending on whether the url can be parsed by the tool youtube -e 'url' prints the stream title and youtube -f 'url' prints the video url. To use this portably, we can use system() and redirect the output to a temporary file, and read the title or URL from the file. Or is popen available on all platforms where ffmpeg runs? Is it a clean enough solution? If yes, I can submit the patch adding this solution. From what I could see, all solutions to parse youtube (including quvi) are based on scripts anyway. -- Gilles. pgpuRmdT_1gda.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Segmentation fault in libquvi.c
On Wed, Apr 08, 2015 at 03:51:36PM +0200, Lukasz Marek wrote: > On 8 April 2015 at 15:37, Gilles Chanteperdrix < > gilles.chanteperd...@xenomai.org> wrote: > > > On Wed, Apr 08, 2015 at 02:40:52PM +0200, Michael Niedermayer wrote: > > > On Wed, Apr 08, 2015 at 10:55:45AM +0200, Gilles Chanteperdrix wrote: > > > > Hi, > > > > > > > > I just triend libquvi, and get a segmentation fault in the > > > > libquvi_read_header function, because ff_copy_whitelists is called > > > > before qc->fmtctx is allocated by avformat_open_input. I added a > > > > call to avformat_alloc_context() before ff_copy_whitelists and the > > > > libquvi demuxer works. > > > > > > > > However, I wonder how to fix this properly: the error handling > > > > labels look backward, so that I am not sure where to free the > > > > allocated context in case of error. > > > > > > applied this, yes its correct > > > > Ok, there are other details missing, the stream does not get a > > duration, start_time and bitrate. This can easily be fixed, but as > > wm4 said libquvi seems an abandoned project. > > > > Would there be any interest in a solution based on youtube-dl? It > > seems to be the standard, these days. I just ran a few tests: > > > > youtube -qs 'url' returns 0 or 1 depending on whether the url can be > > parsed by the tool > > > > youtube -e 'url' prints the stream title > > > > and youtube -f 'url' prints the video url. > > > > To use this portably, we can use system() and redirect the > > output to a temporary file, and read the title or URL from the file. > > Or is popen available on all platforms where ffmpeg runs? > > > > Is it a clean enough solution? If yes, I can submit the patch adding > > this solution. From what I could see, all solutions to parse youtube > > (including quvi) are based on scripts anyway. > > > > I submitted a patch more that year ago to support youtube natively, as I > remember this was based on the script you mention, but it was rejected > because we have libquvi. [sic!] > Project not actively developed is not the same as project not working. > In corner case it is not developed because everything is working > perfectly. libquvi seems to work on youtube yes, but not on daily motion and other streaming sites I tried where youtube-dl works, it simply does not work. The sites change, so the tool to parse them must be up to date to work correctly. So no, libquvi does not "work perfectly". And libquvi ffmpeg demuxer does not seem to be in a very good shape either, it simply segfaults, and even when the segfault is resolved, has other issues, the extern AVFormatcontext fields are simply not filled. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Segmentation fault in libquvi.c
On Wed, Apr 08, 2015 at 04:13:58PM +0200, wm4 wrote: > On Wed, 8 Apr 2015 15:37:03 +0200 > Gilles Chanteperdrix wrote: > > > On Wed, Apr 08, 2015 at 02:40:52PM +0200, Michael Niedermayer wrote: > > > On Wed, Apr 08, 2015 at 10:55:45AM +0200, Gilles Chanteperdrix wrote: > > > > Hi, > > > > > > > > I just triend libquvi, and get a segmentation fault in the > > > > libquvi_read_header function, because ff_copy_whitelists is called > > > > before qc->fmtctx is allocated by avformat_open_input. I added a > > > > call to avformat_alloc_context() before ff_copy_whitelists and the > > > > libquvi demuxer works. > > > > > > > > However, I wonder how to fix this properly: the error handling > > > > labels look backward, so that I am not sure where to free the > > > > allocated context in case of error. > > > > > > applied this, yes its correct > > > > Ok, there are other details missing, the stream does not get a > > duration, start_time and bitrate. This can easily be fixed, but as > > wm4 said libquvi seems an abandoned project. > > > > Would there be any interest in a solution based on youtube-dl? It > > seems to be the standard, these days. I just ran a few tests: > > > > youtube -qs 'url' returns 0 or 1 depending on whether the url can be > > parsed by the tool > > > > youtube -e 'url' prints the stream title > > > > and youtube -f 'url' prints the video url. > > > > To use this portably, we can use system() and redirect the > > output to a temporary file, and read the title or URL from the file. > > Or is popen available on all platforms where ffmpeg runs? > > > > Is it a clean enough solution? If yes, I can submit the patch adding > > this solution. From what I could see, all solutions to parse youtube > > (including quvi) are based on scripts anyway. > > > > Calling external processes from within libavformat doesn't sound like > the right thing to do. You can do it perfectly fine in a higher level, > of course (I'm doing this too). The point is: the youtube-dl developers are doing a terrific job maintaining their software and supporting a lot of sites: run youtube-dl --list-extractors to be convinced. So, I do not think it makes sense to duplicate this effort "at a higher level", this is a waste of ressources. Maybe ask them kindly to provide us with some command line arguments that would arrange us would make sense, though. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Segmentation fault in libquvi.c
On Wed, Apr 08, 2015 at 04:26:20PM +0200, Michael Niedermayer wrote: > On Wed, Apr 08, 2015 at 04:01:38PM +0200, Gilles Chanteperdrix wrote: > > On Wed, Apr 08, 2015 at 03:51:36PM +0200, Lukasz Marek wrote: > > > On 8 April 2015 at 15:37, Gilles Chanteperdrix < > > > gilles.chanteperd...@xenomai.org> wrote: > > > > > > > On Wed, Apr 08, 2015 at 02:40:52PM +0200, Michael Niedermayer wrote: > > > > > On Wed, Apr 08, 2015 at 10:55:45AM +0200, Gilles Chanteperdrix wrote: > > > > > > Hi, > > > > > > > > > > > > I just triend libquvi, and get a segmentation fault in the > > > > > > libquvi_read_header function, because ff_copy_whitelists is called > > > > > > before qc->fmtctx is allocated by avformat_open_input. I added a > > > > > > call to avformat_alloc_context() before ff_copy_whitelists and the > > > > > > libquvi demuxer works. > > > > > > > > > > > > However, I wonder how to fix this properly: the error handling > > > > > > labels look backward, so that I am not sure where to free the > > > > > > allocated context in case of error. > > > > > > > > > > applied this, yes its correct > > > > > > > > Ok, there are other details missing, the stream does not get a > > > > duration, start_time and bitrate. This can easily be fixed, but as > > > > wm4 said libquvi seems an abandoned project. > > > > > > > > Would there be any interest in a solution based on youtube-dl? It > > > > seems to be the standard, these days. I just ran a few tests: > > > > > > > > youtube -qs 'url' returns 0 or 1 depending on whether the url can be > > > > parsed by the tool > > > > > > > > youtube -e 'url' prints the stream title > > > > > > > > and youtube -f 'url' prints the video url. > > > > > > > > To use this portably, we can use system() and redirect the > > > > output to a temporary file, and read the title or URL from the file. > > > > Or is popen available on all platforms where ffmpeg runs? > > > > > > > > Is it a clean enough solution? If yes, I can submit the patch adding > > > > this solution. From what I could see, all solutions to parse youtube > > > > (including quvi) are based on scripts anyway. > > > > > > > > > > I submitted a patch more that year ago to support youtube natively, as I > > > remember this was based on the script you mention, but it was rejected > > > because we have libquvi. [sic!] > > > Project not actively developed is not the same as project not working. > > > In corner case it is not developed because everything is working > > > perfectly. > > > > libquvi seems to work on youtube yes, but not on daily motion and > > other streaming sites I tried where youtube-dl works, it simply does > > not work. The sites change, so the tool to parse them must be up to > > date to work correctly. So no, libquvi does not "work perfectly". > > > > And libquvi ffmpeg demuxer does not seem to be in a very good shape > > either, it simply segfaults, and even when the segfault is resolved, > > has other issues, the extern AVFormatcontext fields are simply not > > filled. > > the whitelist segfault is already fixed > and it worked with git master (download from youtube) today > > maybe someone wants to take over libquvi maintaince ? i mean libquvi > not our wraper. ive seen there are patches on teh libquvi developer > mailing list which supposedly fix some of the not working services I think the set of sites supported by libquvi is really small compared to the ones supported by youtube-dl, and even smaller the site that actually work, because they have changed and quvi did not change. > > but really iam happy with any solution that works and that people > like, libquvi, youtube-dl, anythng else > > but a plain ./ffplay http://youtube.com/... > should work ideally > or less ideally with a forced demuxer -f xxx http://youtube.com/... Meanwhile, I get that solution working, however, there is a catch: youtube-dl has a "generic" extractor which accepts plain http urls to videos, so, when the input format opens the nested import formats, the probe function succeeds again, and we have an infinite recursion, I have not found a way to disable this extractor on the command line, and have to admit that my ffmpeg knowledge is not sufficient at this point, the only solution I have found to prevent recursion is to return 0 in the probe function all the time, and only be able to use the format with -f. Shall I send the patch, or could you explain me how to avoid this ? -- Gilles. pgpg1OIwMtKqq.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Segmentation fault in libquvi.c
On Wed, Apr 08, 2015 at 05:29:13PM +0200, wm4 wrote: > On Wed, 8 Apr 2015 17:22:58 +0200 > Gilles Chanteperdrix wrote: > > > On Wed, Apr 08, 2015 at 04:13:58PM +0200, wm4 wrote: > > > On Wed, 8 Apr 2015 15:37:03 +0200 > > > Gilles Chanteperdrix wrote: > > > > > > > On Wed, Apr 08, 2015 at 02:40:52PM +0200, Michael Niedermayer wrote: > > > > > On Wed, Apr 08, 2015 at 10:55:45AM +0200, Gilles Chanteperdrix wrote: > > > > > > Hi, > > > > > > > > > > > > I just triend libquvi, and get a segmentation fault in the > > > > > > libquvi_read_header function, because ff_copy_whitelists is called > > > > > > before qc->fmtctx is allocated by avformat_open_input. I added a > > > > > > call to avformat_alloc_context() before ff_copy_whitelists and the > > > > > > libquvi demuxer works. > > > > > > > > > > > > However, I wonder how to fix this properly: the error handling > > > > > > labels look backward, so that I am not sure where to free the > > > > > > allocated context in case of error. > > > > > > > > > > applied this, yes its correct > > > > > > > > Ok, there are other details missing, the stream does not get a > > > > duration, start_time and bitrate. This can easily be fixed, but as > > > > wm4 said libquvi seems an abandoned project. > > > > > > > > Would there be any interest in a solution based on youtube-dl? It > > > > seems to be the standard, these days. I just ran a few tests: > > > > > > > > youtube -qs 'url' returns 0 or 1 depending on whether the url can be > > > > parsed by the tool > > > > > > > > youtube -e 'url' prints the stream title > > > > > > > > and youtube -f 'url' prints the video url. > > > > > > > > To use this portably, we can use system() and redirect the > > > > output to a temporary file, and read the title or URL from the file. > > > > Or is popen available on all platforms where ffmpeg runs? > > > > > > > > Is it a clean enough solution? If yes, I can submit the patch adding > > > > this solution. From what I could see, all solutions to parse youtube > > > > (including quvi) are based on scripts anyway. > > > > > > > > > > Calling external processes from within libavformat doesn't sound like > > > the right thing to do. You can do it perfectly fine in a higher level, > > > of course (I'm doing this too). > > > > The point is: the youtube-dl developers are doing a terrific job > > maintaining their software and supporting a lot of sites: > > run youtube-dl --list-extractors to be convinced. So, I do not think > > it makes sense to duplicate this effort "at a higher level", this is > > a waste of ressources. Maybe ask them kindly to provide us with some > > command line arguments that would arrange us would make sense, > > though. > > > > I'm not talking about duplicating this. I'm saying that you should put > calling youtube-dl not into libavformat, but into the application which > uses libavformat. So, everyone should duplicate the job of using youtube-dl instead of having it done once and for all in a common library ? -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] youtube patches
Enough talking, here are some patches, and I will let the maintainers decide. On the libquvi side, the first patch cleans up the error handling in the libquvi_read_header function to avoid calling libquvi cleanup functions on uninitialized variables. The second patch adds a call to avformat_find_stream_info for the inner format context so as to be able to fill the start_time, duration and bit_rate of the outer format context. On the youtube-dl side, a patch adds support for a youtube-dl based demuxer, which is functionally equivalent to libquvi, except that it supports all the sites supported by youtube-dl. I avoided using popen on purpose, so as to try and be as much cross-platform as possible, though the av_tempfile function returns a file descriptor, so, probably requires UNIX or POSIX anyway. In my opinion, it does not make sense to maintain both solutions. Regards. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/3] avformat/libquvi: copy inner format context fields to outer format context
Signed-off-by: Gilles Chanteperdrix --- libavformat/libquvi.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/libavformat/libquvi.c b/libavformat/libquvi.c index e838b34..8673cb4 100644 --- a/libavformat/libquvi.c +++ b/libavformat/libquvi.c @@ -96,6 +96,10 @@ static int libquvi_read_header(AVFormatContext *s) if (ret < 0) goto err_quvi_cleanup; +ret = avformat_find_stream_info(qc->fmtctx, NULL); +if (ret < 0) +goto err_close_input; + rc = quvi_getprop(m, QUVIPROP_PAGETITLE, &pagetitle); if (rc == QUVI_OK) av_dict_set(&s->metadata, "title", pagetitle, 0); @@ -110,6 +114,9 @@ static int libquvi_read_header(AVFormatContext *s) avpriv_set_pts_info(st, ist->pts_wrap_bits, ist->time_base.num, ist->time_base.den); avcodec_copy_context(st->codec, qc->fmtctx->streams[i]->codec); } +s->duration = qc->fmtctx->duration; +s->start_time = qc->fmtctx->start_time; +s->bit_rate = qc->fmtctx->bit_rate; return 0; -- 1.8.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] avformat/libquvi: fix error handling
avoid calling cleanup functions on uninitialized variables
Signed-off-by: Gilles Chanteperdrix
---
libavformat/libquvi.c | 41 +
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/libavformat/libquvi.c b/libavformat/libquvi.c
index 7c5f7a2..e838b34 100644
--- a/libavformat/libquvi.c
+++ b/libavformat/libquvi.c
@@ -63,28 +63,38 @@ static int libquvi_read_header(AVFormatContext *s)
char *media_url, *pagetitle;
rc = quvi_init(&q);
-if (rc != QUVI_OK)
-goto quvi_fail;
+if (rc != QUVI_OK) {
+ av_log(s, AV_LOG_ERROR, "%s\n", quvi_strerror(q, rc));
+ return AVERROR_EXTERNAL;
+}
quvi_setopt(q, QUVIOPT_FORMAT, qc->format);
rc = quvi_parse(q, s->filename, &m);
-if (rc != QUVI_OK)
-goto quvi_fail;
+if (rc != QUVI_OK) {
+ av_log(s, AV_LOG_ERROR, "%s\n", quvi_strerror(q, rc));
+ ret = AVERROR_EXTERNAL;
+goto err_quvi_close;
+}
rc = quvi_getprop(m, QUVIPROP_MEDIAURL, &media_url);
-if (rc != QUVI_OK)
-goto quvi_fail;
+if (rc != QUVI_OK) {
+ av_log(s, AV_LOG_ERROR, "%s\n", quvi_strerror(q, rc));
+ ret = AVERROR_EXTERNAL;
+ goto err_quvi_cleanup;
+}
if (!(qc->fmtctx = avformat_alloc_context()))
-goto quvi_fail;
+goto err_quvi_cleanup;
-if ((ret = ff_copy_whitelists(qc->fmtctx, s)) < 0)
-goto end;
+if ((ret = ff_copy_whitelists(qc->fmtctx, s)) < 0) {
+ avformat_free_context(&qc->fmtctx);
+goto err_quvi_cleanup;
+}
ret = avformat_open_input(&qc->fmtctx, media_url, NULL, NULL);
if (ret < 0)
-goto end;
+goto err_quvi_cleanup;
rc = quvi_getprop(m, QUVIPROP_PAGETITLE, &pagetitle);
if (rc == QUVI_OK)
@@ -95,7 +105,7 @@ static int libquvi_read_header(AVFormatContext *s)
AVStream *ist = qc->fmtctx->streams[i];
if (!st) {
ret = AVERROR(ENOMEM);
-goto end;
+goto err_close_input;
}
avpriv_set_pts_info(st, ist->pts_wrap_bits, ist->time_base.num,
ist->time_base.den);
avcodec_copy_context(st->codec, qc->fmtctx->streams[i]->codec);
@@ -103,12 +113,11 @@ static int libquvi_read_header(AVFormatContext *s)
return 0;
-quvi_fail:
-av_log(s, AV_LOG_ERROR, "%s\n", quvi_strerror(q, rc));
-ret = AVERROR_EXTERNAL;
-
-end:
+ err_close_input:
+avformat_close_input(&qc->fmtctx);
+ err_quvi_cleanup:
quvi_parse_close(&m);
+ err_quvi_close:
quvi_close(&q);
return ret;
}
--
1.8.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
Signed-off-by: Gilles Chanteperdrix
---
libavformat/Makefile | 1 +
libavformat/allformats.c | 1 +
libavformat/youtubedl.c | 221 +++
3 files changed, 223 insertions(+)
create mode 100644 libavformat/youtubedl.c
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 8d9a770..788b505 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -487,6 +487,7 @@ OBJS-$(CONFIG_LIBQUVI_DEMUXER) += libquvi.o
OBJS-$(CONFIG_LIBRTMP) += librtmp.o
OBJS-$(CONFIG_LIBSSH_PROTOCOL) += libssh.o
OBJS-$(CONFIG_LIBSMBCLIENT_PROTOCOL) += libsmbclient.o
+OBJS-$(CONFIG_YOUTUBEDL_DEMUXER) += youtubedl.o
# protocols I/O
OBJS-$(CONFIG_APPLEHTTP_PROTOCOL)+= hlsproto.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index e6a9d01..a684b73 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -386,6 +386,7 @@ void av_register_all(void)
REGISTER_DEMUXER (LIBMODPLUG, libmodplug);
REGISTER_MUXDEMUX(LIBNUT, libnut);
REGISTER_DEMUXER (LIBQUVI, libquvi);
+REGISTER_DEMUXER (YOUTUBEDL, youtubedl);
REGISTER_PROTOCOL(LIBRTMP, librtmp);
REGISTER_PROTOCOL(LIBRTMPE, librtmpe);
REGISTER_PROTOCOL(LIBRTMPS, librtmps);
diff --git a/libavformat/youtubedl.c b/libavformat/youtubedl.c
new file mode 100644
index 000..2d4ec60
--- /dev/null
+++ b/libavformat/youtubedl.c
@@ -0,0 +1,221 @@
+/*
+ * Copyright (c) 2015 Gilles Chanteperdrix
+ *
+ * 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
+ */
+
+#include
+#include
+
+#include "libavformat/avformat.h"
+#include "libavformat/internal.h"
+#include "libavutil/avassert.h"
+#include "libavutil/avstring.h"
+#include "libavutil/file.h"
+#include "libavutil/opt.h"
+
+typedef struct {
+const AVClass *class;
+char *format;
+AVFormatContext *fmtctx;
+} YoutubeDlContext;
+
+#define OUTBUF_SIZE 2048
+#define OFFSET(x) offsetof(YoutubeDlContext, x)
+#define FLAGS AV_OPT_FLAG_DECODING_PARAM
+static const AVOption youtubedl_options[] = {
+{ "format", "request specific format", OFFSET(format), AV_OPT_TYPE_STRING,
{.str="best"}, .flags = FLAGS },
+{ NULL }
+};
+
+static const AVClass youtubedl_context_class = {
+.class_name = "youtubedl",
+.item_name = av_default_item_name,
+.option = youtubedl_options,
+.version= LIBAVUTIL_VERSION_INT,
+};
+
+static int youtubedl_close(AVFormatContext *s)
+{
+YoutubeDlContext *yc = s->priv_data;
+if (yc->fmtctx)
+avformat_close_input(&yc->fmtctx);
+return 0;
+}
+
+static int youtubedl_get_output(char *output, size_t output_size,
+ AVFormatContext *s, const char *command)
+{
+char *output_filename;
+int fd, ret, eof;
+size_t line_len;
+FILE *f;
+
+fd = av_tempfile("youtubedl", &output_filename, 0, s);
+if (fd < 0) {
+ av_log(s, AV_LOG_ERROR, "Failed to create temporary file\n");
+ return fd;
+}
+close(fd);
+
+command = av_asprintf("%s > %s", command, output_filename);
+if (!command) {
+ av_log(s, AV_LOG_ERROR, "Out of memory\n");
+ ret = AVERROR(ENOMEM);
+ goto err_unlink_output;
+}
+
+ret = system(command);
+av_free((void *)command);
+
+if (ret != 0) {
+ av_log(s, AV_LOG_ERROR, "Unexpected youtube-dl error\n");
+ ret = ret < 0 ? AVERROR(errno) : AVERROR_EXTERNAL;
+ goto err_unlink_output;
+}
+
+f = fopen(output_filename, "rb");
+if (!f) {
+ av_log(s, AV_LOG_ERROR, "Unexpected error opening youtube-dl output\n");
+ ret = AVERROR(errno);
+ goto err_unlink_output;
+}
+
+eof = !fgets(output, output_size, f);
+fclose(f);
+if (eof) {
+ if (s)
+ av_log(s, AV_LOG_ERROR, "Empty youtube-dl output\n");
+ ret = AVERROR_INVALIDDATA;
+ goto err_unlink_output;
+}
+
+line_len = strcspn(output, "\r\n");
+
Re: [FFmpeg-devel] youtube patches
On Wed, Apr 08, 2015 at 07:15:51PM +0200, Gilles Chanteperdrix wrote: > Enough talking, here are some patches, and I will let the > maintainers decide. > > On the libquvi side, the first patch cleans up the error handling in > the libquvi_read_header function to avoid calling libquvi cleanup > functions on uninitialized variables. The second patch adds a call > to avformat_find_stream_info for the inner format context so as to > be able to fill the start_time, duration and bit_rate of the outer > format context. > > On the youtube-dl side, a patch adds support for a youtube-dl based > demuxer, which is functionally equivalent to libquvi, except that it > supports all the sites supported by youtube-dl. I avoided using > popen on purpose, so as to try and be as much cross-platform as > possible, though the av_tempfile function returns a file descriptor, > so, probably requires UNIX or POSIX anyway. > > In my opinion, it does not make sense to maintain both solutions. A related question: is it possible for an application to register its own input formats in addition to those of the avformat library ? -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 07:24:27PM +0200, wm4 wrote:
> On Wed, 8 Apr 2015 19:16:38 +0200
> Gilles Chanteperdrix wrote:
>
> > Signed-off-by: Gilles Chanteperdrix
> > ---
> > libavformat/Makefile | 1 +
> > libavformat/allformats.c | 1 +
> > libavformat/youtubedl.c | 221
> > +++
> > 3 files changed, 223 insertions(+)
> > create mode 100644 libavformat/youtubedl.c
> >
> > diff --git a/libavformat/Makefile b/libavformat/Makefile
> > index 8d9a770..788b505 100644
> > --- a/libavformat/Makefile
> > +++ b/libavformat/Makefile
> > @@ -487,6 +487,7 @@ OBJS-$(CONFIG_LIBQUVI_DEMUXER) += libquvi.o
> > OBJS-$(CONFIG_LIBRTMP) += librtmp.o
> > OBJS-$(CONFIG_LIBSSH_PROTOCOL) += libssh.o
> > OBJS-$(CONFIG_LIBSMBCLIENT_PROTOCOL) += libsmbclient.o
> > +OBJS-$(CONFIG_YOUTUBEDL_DEMUXER) += youtubedl.o
> >
> > # protocols I/O
> > OBJS-$(CONFIG_APPLEHTTP_PROTOCOL)+= hlsproto.o
> > diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> > index e6a9d01..a684b73 100644
> > --- a/libavformat/allformats.c
> > +++ b/libavformat/allformats.c
> > @@ -386,6 +386,7 @@ void av_register_all(void)
> > REGISTER_DEMUXER (LIBMODPLUG, libmodplug);
> > REGISTER_MUXDEMUX(LIBNUT, libnut);
> > REGISTER_DEMUXER (LIBQUVI, libquvi);
> > +REGISTER_DEMUXER (YOUTUBEDL, youtubedl);
> > REGISTER_PROTOCOL(LIBRTMP, librtmp);
> > REGISTER_PROTOCOL(LIBRTMPE, librtmpe);
> > REGISTER_PROTOCOL(LIBRTMPS, librtmps);
> > diff --git a/libavformat/youtubedl.c b/libavformat/youtubedl.c
> > new file mode 100644
> > index 000..2d4ec60
> > --- /dev/null
> > +++ b/libavformat/youtubedl.c
> > @@ -0,0 +1,221 @@
> > +/*
> > + * Copyright (c) 2015 Gilles Chanteperdrix
> > + *
> > + * 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
> > + */
> > +
> > +#include
> > +#include
> > +
> > +#include "libavformat/avformat.h"
> > +#include "libavformat/internal.h"
> > +#include "libavutil/avassert.h"
> > +#include "libavutil/avstring.h"
> > +#include "libavutil/file.h"
> > +#include "libavutil/opt.h"
> > +
> > +typedef struct {
> > +const AVClass *class;
> > +char *format;
> > +AVFormatContext *fmtctx;
> > +} YoutubeDlContext;
> > +
> > +#define OUTBUF_SIZE 2048
> > +#define OFFSET(x) offsetof(YoutubeDlContext, x)
> > +#define FLAGS AV_OPT_FLAG_DECODING_PARAM
> > +static const AVOption youtubedl_options[] = {
> > +{ "format", "request specific format", OFFSET(format),
> > AV_OPT_TYPE_STRING, {.str="best"}, .flags = FLAGS },
> > +{ NULL }
> > +};
> > +
> > +static const AVClass youtubedl_context_class = {
> > +.class_name = "youtubedl",
> > +.item_name = av_default_item_name,
> > +.option = youtubedl_options,
> > +.version= LIBAVUTIL_VERSION_INT,
> > +};
> > +
> > +static int youtubedl_close(AVFormatContext *s)
> > +{
> > +YoutubeDlContext *yc = s->priv_data;
> > +if (yc->fmtctx)
> > +avformat_close_input(&yc->fmtctx);
> > +return 0;
> > +}
> > +
> > +static int youtubedl_get_output(char *output, size_t output_size,
> > + AVFormatContext *s, const char *command)
> > +{
> > +char *output_filename;
> > +int fd, ret, eof;
> > +size_t line_len;
> > +FILE *f;
> > +
> > +fd = av_tempfile("youtubedl"
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 07:30:12PM +0200, wm4 wrote:
> On Wed, 8 Apr 2015 19:27:55 +0200
> Gilles Chanteperdrix wrote:
>
> > On Wed, Apr 08, 2015 at 07:24:27PM +0200, wm4 wrote:
> > > On Wed, 8 Apr 2015 19:16:38 +0200
> > > Gilles Chanteperdrix wrote:
> > >
> > > > Signed-off-by: Gilles Chanteperdrix
> > > > ---
> > > > libavformat/Makefile | 1 +
> > > > libavformat/allformats.c | 1 +
> > > > libavformat/youtubedl.c | 221
> > > > +++
> > > > 3 files changed, 223 insertions(+)
> > > > create mode 100644 libavformat/youtubedl.c
> > > >
> > > > diff --git a/libavformat/Makefile b/libavformat/Makefile
> > > > index 8d9a770..788b505 100644
> > > > --- a/libavformat/Makefile
> > > > +++ b/libavformat/Makefile
> > > > @@ -487,6 +487,7 @@ OBJS-$(CONFIG_LIBQUVI_DEMUXER) +=
> > > > libquvi.o
> > > > OBJS-$(CONFIG_LIBRTMP) += librtmp.o
> > > > OBJS-$(CONFIG_LIBSSH_PROTOCOL) += libssh.o
> > > > OBJS-$(CONFIG_LIBSMBCLIENT_PROTOCOL) += libsmbclient.o
> > > > +OBJS-$(CONFIG_YOUTUBEDL_DEMUXER) += youtubedl.o
> > > >
> > > > # protocols I/O
> > > > OBJS-$(CONFIG_APPLEHTTP_PROTOCOL)+= hlsproto.o
> > > > diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> > > > index e6a9d01..a684b73 100644
> > > > --- a/libavformat/allformats.c
> > > > +++ b/libavformat/allformats.c
> > > > @@ -386,6 +386,7 @@ void av_register_all(void)
> > > > REGISTER_DEMUXER (LIBMODPLUG, libmodplug);
> > > > REGISTER_MUXDEMUX(LIBNUT, libnut);
> > > > REGISTER_DEMUXER (LIBQUVI, libquvi);
> > > > +REGISTER_DEMUXER (YOUTUBEDL, youtubedl);
> > > > REGISTER_PROTOCOL(LIBRTMP, librtmp);
> > > > REGISTER_PROTOCOL(LIBRTMPE, librtmpe);
> > > > REGISTER_PROTOCOL(LIBRTMPS, librtmps);
> > > > diff --git a/libavformat/youtubedl.c b/libavformat/youtubedl.c
> > > > new file mode 100644
> > > > index 000..2d4ec60
> > > > --- /dev/null
> > > > +++ b/libavformat/youtubedl.c
> > > > @@ -0,0 +1,221 @@
> > > > +/*
> > > > + * Copyright (c) 2015 Gilles Chanteperdrix
> > > > + *
> > > > + * 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
> > > > + */
> > > > +
> > > > +#include
> > > > +#include
> > > > +
> > > > +#include "libavformat/avformat.h"
> > > > +#include "libavformat/internal.h"
> > > > +#include "libavutil/avassert.h"
> > > > +#include "libavutil/avstring.h"
> > > > +#include "libavutil/file.h"
> > > > +#include "libavutil/opt.h"
> > > > +
> > > > +typedef struct {
> > > > +const AVClass *class;
> > > > +char *format;
> > > > +AVFormatContext *fmtctx;
> > > > +} YoutubeDlContext;
> > > > +
> > > > +#define OUTBUF_SIZE 2048
> > > > +#define OFFSET(x) offsetof(YoutubeDlContext, x)
> > > > +#define FLAGS AV_OPT_FLAG_DECODING_PARAM
> > > > +static const AVOption youtubedl_options[] = {
> > > > +{ "format", "request specific format", OFFSET(format),
> > > > AV_OPT_TYPE_STRING, {.str="bes
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 07:24:27PM +0200, wm4 wrote:
> > +snprintf(buffer, sizeof(buffer), "youtube-dl -f %s -g '%s'",
> > + yc->format, s->filename);
Ok, missing single quotes here around the format.
--
Gilles.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 07:30:12PM +0200, wm4 wrote:
> On Wed, 8 Apr 2015 19:27:55 +0200
> Gilles Chanteperdrix wrote:
>
> > On Wed, Apr 08, 2015 at 07:24:27PM +0200, wm4 wrote:
> > > On Wed, 8 Apr 2015 19:16:38 +0200
> > > Gilles Chanteperdrix wrote:
> > >
> > > > Signed-off-by: Gilles Chanteperdrix
> > > > ---
> > > > libavformat/Makefile | 1 +
> > > > libavformat/allformats.c | 1 +
> > > > libavformat/youtubedl.c | 221
> > > > +++
> > > > 3 files changed, 223 insertions(+)
> > > > create mode 100644 libavformat/youtubedl.c
> > > >
> > > > diff --git a/libavformat/Makefile b/libavformat/Makefile
> > > > index 8d9a770..788b505 100644
> > > > --- a/libavformat/Makefile
> > > > +++ b/libavformat/Makefile
> > > > @@ -487,6 +487,7 @@ OBJS-$(CONFIG_LIBQUVI_DEMUXER) +=
> > > > libquvi.o
> > > > OBJS-$(CONFIG_LIBRTMP) += librtmp.o
> > > > OBJS-$(CONFIG_LIBSSH_PROTOCOL) += libssh.o
> > > > OBJS-$(CONFIG_LIBSMBCLIENT_PROTOCOL) += libsmbclient.o
> > > > +OBJS-$(CONFIG_YOUTUBEDL_DEMUXER) += youtubedl.o
> > > >
> > > > # protocols I/O
> > > > OBJS-$(CONFIG_APPLEHTTP_PROTOCOL)+= hlsproto.o
> > > > diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> > > > index e6a9d01..a684b73 100644
> > > > --- a/libavformat/allformats.c
> > > > +++ b/libavformat/allformats.c
> > > > @@ -386,6 +386,7 @@ void av_register_all(void)
> > > > REGISTER_DEMUXER (LIBMODPLUG, libmodplug);
> > > > REGISTER_MUXDEMUX(LIBNUT, libnut);
> > > > REGISTER_DEMUXER (LIBQUVI, libquvi);
> > > > +REGISTER_DEMUXER (YOUTUBEDL, youtubedl);
> > > > REGISTER_PROTOCOL(LIBRTMP, librtmp);
> > > > REGISTER_PROTOCOL(LIBRTMPE, librtmpe);
> > > > REGISTER_PROTOCOL(LIBRTMPS, librtmps);
> > > > diff --git a/libavformat/youtubedl.c b/libavformat/youtubedl.c
> > > > new file mode 100644
> > > > index 000..2d4ec60
> > > > --- /dev/null
> > > > +++ b/libavformat/youtubedl.c
> > > > @@ -0,0 +1,221 @@
> > > > +/*
> > > > + * Copyright (c) 2015 Gilles Chanteperdrix
> > > > + *
> > > > + * 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
> > > > + */
> > > > +
> > > > +#include
> > > > +#include
> > > > +
> > > > +#include "libavformat/avformat.h"
> > > > +#include "libavformat/internal.h"
> > > > +#include "libavutil/avassert.h"
> > > > +#include "libavutil/avstring.h"
> > > > +#include "libavutil/file.h"
> > > > +#include "libavutil/opt.h"
> > > > +
> > > > +typedef struct {
> > > > +const AVClass *class;
> > > > +char *format;
> > > > +AVFormatContext *fmtctx;
> > > > +} YoutubeDlContext;
> > > > +
> > > > +#define OUTBUF_SIZE 2048
> > > > +#define OFFSET(x) offsetof(YoutubeDlContext, x)
> > > > +#define FLAGS AV_OPT_FLAG_DECODING_PARAM
> > > > +static const AVOption youtubedl_options[] = {
> > > > +{ "format", "request specific format", OFFSET(format),
> > > > AV_OPT_TYPE_STRING, {.str="bes
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 07:30:48PM +0200, Hendrik Leppkes wrote:
> On Wed, Apr 8, 2015 at 7:27 PM, Gilles Chanteperdrix
> wrote:
> >
> >> Nice security hole.
> >
> > how is that ? I do not see any buffer overflow possible.
> >
>
> Executing a command with system() is very unsafe.
Indeed, never thought of that. With exec this problem does not
exist, we get to choose how the arguments are split.
--
Gilles.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 07:44:13PM +0200, wm4 wrote: > On Wed, 8 Apr 2015 19:39:00 +0200 > Gilles Chanteperdrix wrote: > > > On Wed, Apr 08, 2015 at 07:24:27PM +0200, wm4 wrote: > > > > +snprintf(buffer, sizeof(buffer), "youtube-dl -f %s -g '%s'", > > > > + yc->format, s->filename); > > > > Ok, missing single quotes here around the format. > > > > Doesn't help. You can't fix it. You need to use something other than > system() if you want it to be secure. You can fix it, you can escape the quotes in the string or refuse a string that contains single quotes, but as I said, this starts being cumbersome. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 02:08:21PM -0400, compn wrote: > On Wed, 8 Apr 2015 19:51:12 +0200 > Gilles Chanteperdrix wrote: > > > On Wed, Apr 08, 2015 at 07:44:13PM +0200, wm4 wrote: > > > On Wed, 8 Apr 2015 19:39:00 +0200 > > > Gilles Chanteperdrix wrote: > > > > > > > On Wed, Apr 08, 2015 at 07:24:27PM +0200, wm4 wrote: > > > > > > +snprintf(buffer, sizeof(buffer), "youtube-dl -f %s -g > > > > > > '%s'", > > > > > > + yc->format, s->filename); > > > > > > > > Ok, missing single quotes here around the format. > > > > > > > > > > Doesn't help. You can't fix it. You need to use something other than > > > system() if you want it to be secure. > > > > You can fix it, you can escape the quotes in the string or refuse a > > string that contains single quotes, but as I said, this starts being > > cumbersome. > > > > why not machine translate the python to c and import that into > ffmpeg? :P > > https://github.com/pradyun/Py2C Because then you will have to maintain that C, update it as soon as a youtube-dl python file is updated because a site changed. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 09:07:06PM +0200, Nicolas George wrote:
> Le nonidi 19 germinal, an CCXXIII, Gilles Chanteperdrix a écrit :
> > Signed-off-by: Gilles Chanteperdrix
> > ---
> > libavformat/Makefile | 1 +
> > libavformat/allformats.c | 1 +
> > libavformat/youtubedl.c | 221
> > +++
> > 3 files changed, 223 insertions(+)
> > create mode 100644 libavformat/youtubedl.c
>
> The basic idea seems not bad, but the design is IMHO all wrong.
>
> > +fd = av_tempfile("youtubedl", &output_filename, 0, s);
>
> Do not use a temp file to read the single output of a command. Use a pipe
> directly: more reliable, less garbage to clean up.
As I explained, I did it this way for portability. system is ANSI
not POSIX, redirections work with windows shell too, and av_tempfile
is implemented by ffmpeg, so I expect it to be portable.
>
> > +if (fd < 0) {
>
> > + av_log(s, AV_LOG_ERROR, "Failed to create temporary file\n");
>
> Indentation is odd.
Probably tabs on my side.
>
> > +command = av_asprintf("%s > %s", command, output_filename);
>
> Never EVER do that. Do not try to quote arguments. You will get it wrong.
> Everybody gets it wrong.
Well, actually, it looks not so hard to me. One argument is an URL
so, we could url encode it before passing it to youtube-dl, and the
other is a format which is expected to be something like best,
bestvideo, bestaudio, or some numbers separated by slashes, so, we
can probably add sanity checks on the string. But I get it, system()
is a bad idea in general.
>
> DO NOT USE A SHELL.
>
> Use fork() + exec() directly to execute exactly the command you want. Even
> better, use posix_spawn(), it takes care of a lot of details that people
> usually get wrong with fork+exec.
Well, actually, vfork() + exec() is better than fork() + exec(),
both performance wise and because it works on machines without an
MMU, and the implementation of posix_spawn() in glibc is too
conservative and uses fork() + exec() even in situations where
vfork() could be used, see:
https://sourceware.org/bugzilla/show_bug.cgi?id=10354
So, better use vfork() + exec() (that is what system does for
instance). I did not use it simply because I thought ffmpeg could
have to be compiled on machines without the POSIX interface.
>
> If you really need a shell (you DO NOT need a shell just to establish a
> pipeline or a redirect), then use a constant command and shell command-line
> arguments ($1, $2...) for the variable parts.
>
> > +snprintf(buffer, sizeof(buffer), "youtube-dl -e '%s'", s->filename);
> > +ret = youtubedl_get_output(buffer, sizeof(buffer), s, buffer);
>
> > +snprintf(buffer, sizeof(buffer), "youtube-dl -f %s -g '%s'",
> > + yc->format, s->filename);
> > +ret = youtubedl_get_output(buffer, sizeof(buffer), s, buffer);
>
> I would say: go propose a patch to youtube-dl to make its output fully
> usable in a single call. Well, actually, it already exists.
>
> Then I suggest you just endeavour to write a pseudo-demuxer for the output
> of "youtube-dl -J". No need of a shell or an external command.
>
> > + err_close_input:
> > +avformat_close_input(&yc->fmtctx);
> > + err_free_media_url:
> > +av_free(media_url);
> > + err_free_pagetitle:
> > +av_free(pagetitle);
> > +return ret;
>
> Remember that all these operations are harmless if the corresponding
> variables are initialized to NULL, so you do not need to have that many exit
> points.
A habit.
>
> > + err_free_context:
> > +avformat_free_context(yc->fmtctx);
> > +goto err_free_media_url;
>
> That looks like spaghetti code.
Yes, this is called in only one place, so better put it there.
--
Gilles.
pgpxXC4mQA957.pgp
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] avformat/libquvi: fix error handling
On Wed, Apr 08, 2015 at 09:13:11PM +0200, Michael Niedermayer wrote:
> On Wed, Apr 08, 2015 at 07:16:36PM +0200, Gilles Chanteperdrix wrote:
> > avoid calling cleanup functions on uninitialized variables
> >
> > Signed-off-by: Gilles Chanteperdrix
> > ---
> > libavformat/libquvi.c | 41 +
> > 1 file changed, 25 insertions(+), 16 deletions(-)
> >
> > diff --git a/libavformat/libquvi.c b/libavformat/libquvi.c
> > index 7c5f7a2..e838b34 100644
> > --- a/libavformat/libquvi.c
> > +++ b/libavformat/libquvi.c
> > @@ -63,28 +63,38 @@ static int libquvi_read_header(AVFormatContext *s)
> > char *media_url, *pagetitle;
> >
> > rc = quvi_init(&q);
> > -if (rc != QUVI_OK)
> > -goto quvi_fail;
> > +if (rc != QUVI_OK) {
> > + av_log(s, AV_LOG_ERROR, "%s\n", quvi_strerror(q, rc));
> > + return AVERROR_EXTERNAL;
> > +}
>
> is quvi_strerror() thread safe ?
I do not know, it was called in the original code, simply moved it
around.
Looking at the code, it would seem it is reentrant, with the state
in quvi_t, so as long as the quvi_t is not moved around thread, this
should be thread-safe.
char *quvi_strerror(quvi_t session, QUVIcode code)
{
static const char *errormsgs[] =
{
"no error",
"memory allocation failed",
"bad handle argument to function",
"invalid argument to function",
"curl initialization failed",
"end of list iteration",
"aborted by callback",
"lua initilization failed",
"lua website scripts not found",
"lua util scripts not found",
"invalid error code (internal _INTERNAL_QUVI_LAST)"
};
_quvi_t quvi = (_quvi_t) session;
if (quvi)
{
if (code > _INTERNAL_QUVI_LAST)
return (quvi->errmsg);
}
else
{
if (code > _INTERNAL_QUVI_LAST)
code = _INTERNAL_QUVI_LAST;
}
return ((char *)errormsgs[code]);
}
--
Gilles.
pgpkYqJZ3BNuZ.pgp
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 09:59:17PM +0200, Nicolas George wrote: > Le nonidi 19 germinal, an CCXXIII, Gilles Chanteperdrix a écrit : > > As I explained, I did it this way for portability. system is ANSI > > not POSIX, redirections work with windows shell too, and av_tempfile > > is implemented by ffmpeg, so I expect it to be portable. > > Implementing pipe reading for other systems looks like a better time > investment. > > > Well, actually, it looks not so hard to me. > > Every person who got it wrong thought that too. > > > One argument is an URL so, we could url encode it before passing it to > > youtube-dl > > No, you can not, because it is probably already URL-encoded. > > > Well, actually, vfork() + exec() is better than fork() + exec(), > > both performance wise and because it works on machines without an > > MMU, and the implementation of posix_spawn() in glibc is too > > conservative and uses fork() + exec() even in situations where > > vfork() could be used, see: > > https://sourceware.org/bugzilla/show_bug.cgi?id=10354 > > > > So, better use vfork() + exec() (that is what system does for > > instance). I did not use it simply because I thought ffmpeg could > > have to be compiled on machines without the POSIX interface. > > That was arguments for 20 years ago. Nowadays, vfork() was left to bitrot > for compatibility reasons while fork() was insanely optimized. And in the > meantime, threads were invented, with their own set of problems when it > comes to spawning a child process. That is what the doc says, but the doc is wrong. Measurements show that fork() has a detrimental effect, not as much at the moment of the fork, this is true, this instant is short, but later: all private writable mappings in the parent process are made read-only and need to take a fault and go to kernel-space to re-change the MMU configuration. You can make a simple experiment, map a large private anonymous area (say 512 MB), and measure the time it takes to write one byte on each page before and after a fork. At least the difference was huge last time I tested. And again, on a platform without MMU, you have vfork(), not fork(). So, when you just need to run exec, it is stupid > > So the short of it: use posix_spawn(). > > The way it is implemented is at best not your concern, at worst premature > optimization. > > > I did not use it simply because I thought ffmpeg could have to be compiled > > on machines without the POSIX interface. > > FFmpeg requires POSIX. posix_spawn() is a POSIX option, yet, so it requires > a configure check. But apart from that, let someone who cares implement for > other systems. > > Anyway: > > > > Then I suggest you just endeavour to write a pseudo-demuxer for the output > > > of "youtube-dl -J". No need of a shell or an external command. > > Just do that, no need for any fancy system call for that, and it would serve > as a base for a more fully automatic version anyway later. I do not follow you here. youtube-dl -J has a uselessly verbose output, much harder to parse than youtube-dl -g which gives you just one URL. But you still need to run youtube-dl. So, I do not see the upside. -- Gilles. pgp4T9wkD6zIe.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 08:52:12PM +0100, Derek Buitenhuis wrote: > On 4/8/2015 6:16 PM, Gilles Chanteperdrix wrote: > > Signed-off-by: Gilles Chanteperdrix > > --- > > libavformat/Makefile | 1 + > > libavformat/allformats.c | 1 + > > libavformat/youtubedl.c | 221 > > +++ > > 3 files changed, 223 insertions(+) > > create mode 100644 libavformat/youtubedl.c > > I must say this is incredibly dumb and has no place at all in libav*format*. > This > should be done in the layer above whatever avformat (and not even to mention > the > hilarity of running a cli script from a library). > > I am registering my dissent here, in writing, so I can point to it in the > future > when asked (and I always get asked). > > NOTE: I am sure people will respond and explain how avformat is totally the > right > place, just like it was the right place for a dir listing api, and an http > server, > and probably soon systemd. I am not going to respond, or otherwise engage in > mailing list flamewars. This mail is read-only. For my defense: I asked if it would not be to ugly before starting and (mis)writing it. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 10:08:25PM +0200, wm4 wrote: > On Wed, 08 Apr 2015 20:52:12 +0100 > Derek Buitenhuis wrote: > > > On 4/8/2015 6:16 PM, Gilles Chanteperdrix wrote: > > > Signed-off-by: Gilles Chanteperdrix > > > --- > > > libavformat/Makefile | 1 + > > > libavformat/allformats.c | 1 + > > > libavformat/youtubedl.c | 221 > > > +++ > > > 3 files changed, 223 insertions(+) > > > create mode 100644 libavformat/youtubedl.c > > > > I must say this is incredibly dumb and has no place at all in > > libav*format*. This > > should be done in the layer above whatever avformat (and not even to > > mention the > > hilarity of running a cli script from a library). > > > > I am registering my dissent here, in writing, so I can point to it in the > > future > > when asked (and I always get asked). > > > > NOTE: I am sure people will respond and explain how avformat is totally the > > right > > place, just like it was the right place for a dir listing api, and an http > > server, > > and probably soon systemd. I am not going to respond, or otherwise engage in > > mailing list flamewars. This mail is read-only. > > +1 > > As usual, people are trying to force things that don't belong into a > low level API, just because the low level API is the only API in ffmpeg. For my defense again: the libquvi demuxer caused a segmentation fault, I posted here, you answered that the project was dead, so I looked for a replacement. I am not trying to force anything. I am ready to forget all about these patches. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 04:31:34PM -0400, compn wrote:
> On Wed, 8 Apr 2015 22:26:23 +0200
> Gilles Chanteperdrix wrote:
>
> > ready to forget all about these patches.
>
> thanks for the patches, even if some people do not like them.
>
> please dont be discouraged by strongly worded mails.
>
> its a long term bikeshed that you have stepped in. the developers are
> mostly shouting into the wind around here about what should be in each
> library and what should not.
I am not discouraged, it is better to see a lively mailing list than
one where people do not care, but I will pass on these patches, I am
not very much interested by the functionality, I agree that calling
the script from the demuxer was a bad idea, I was just looking for a
quick solution, and have other things to do.
--
Gilles.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 10:37:13PM +0200, Gilles Chanteperdrix wrote: > On Wed, Apr 08, 2015 at 04:31:34PM -0400, compn wrote: > > On Wed, 8 Apr 2015 22:26:23 +0200 > > Gilles Chanteperdrix wrote: > > > > > ready to forget all about these patches. > > > > thanks for the patches, even if some people do not like them. > > > > please dont be discouraged by strongly worded mails. > > > > its a long term bikeshed that you have stepped in. the developers are > > mostly shouting into the wind around here about what should be in each > > library and what should not. > > I am not discouraged, it is better to see a lively mailing list than > one where people do not care, but I will pass on these patches, I am > not very much interested by the functionality, I agree that calling > the script from the demuxer was a bad idea, I was just looking for a > quick solution, and have other things to do. And forgot to add my apologies for these bad patches and the noise that followed. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Thu, Apr 09, 2015 at 10:59:24PM +0200, Lukasz Marek wrote: > On 09.04.2015 13:50, Kieran Kunhya wrote: > >On 9 April 2015 at 12:17, Rodger Combs wrote: > >>Agreed, this belongs in a higher layer. I think it'd be reasonable for > >>FFmpeg to have a higher-layer library handling things like playlists and > >>this, but that's another conversation. > > > >+1 > > So instead of complaining and spamming with "+1", prepare a patch and submit > it? Community will discuss it and do what's the best for the project (and > yes, project is not single person who has their wishes). Personally I woulnt > complain about that separation if done properly, but just don't see a reason > for it. Most of the stuff you don't need/like you can disable at compilation > level. > We have separation libavformat/libavdevice and most of patches I submitted > regarding libavdevice were complained by one person. > > Sometimes I have a feeling that ffmpeg is an open source project to feed > wm4's project. Everything he doesn't need is "retarded", "rotten", "dumb", > etc, etc. > > I don't want to be drama queen here, but the way author of this patch was > treated is bellow any level of dignity. I assure you the author of this patch is not vexed or discouraged. My main interest in using ffmpeg is an an RTSP client library (ok, maybe some say this should not be a demuxer either ?), the youtube-dl thing would just have been a nice addition if it had been possible to have a quick solution, but this is nothing I care to spend to much time on. My main effort for free-software go to another project which has less maintainers, contributors and users than ffmpeg. Now for the "high level API", if you mind the opinion of someone which has a rather new view on this project and is implementing a simplistic player, I believe what could be done relatively easy is to split ffplay into say libffplay which would implement a high-level API for players with calls such as open, play, pause, seek, close, and ffplay which would use this API and implement the human machine interface. This would make implementing a custom player relatively easy by using libffplay being free to use GTK or Qt, or whatever else for the GUI. Maybe the same goes for the ffmpeg tool. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Wed, Apr 08, 2015 at 10:22:42PM +0200, Nicolas George wrote: > Le nonidi 19 germinal, an CCXXIII, Gilles Chanteperdrix a écrit : > > So, when you just need to run exec, it is stupid > > Exactly what I wrote: not your problem / premature optimization. > > > I do not follow you here. youtube-dl -J has a uselessly verbose > > output, much harder to parse than youtube-dl -g which gives you just > > one URL. But you still need to run youtube-dl. So, I do not see the > > upside. > > Scratch "uselessly" and you will get it right: -J gives you everything you > may need to download the video correctly. -g only gives you one piece of > information, which is sometimes sufficient (but not not enough that you do > not feel the need to call a second time for -e) but only for a few sites. > > So here is the upside of -J: it works. > > Second upside: it works with a single call. > > Third upside: since the information is completely self-contained, you can > pass it around easily. People here whine about "ffmpeg -f youtubedl", and > they have valid arguments. This: > > youtube-dl -J $url | ffmpeg -f youtubedl -i - > > is just as easy, and nobody will whine. Well, in that case, ffmpeg -i `youtube-dl -f bestvideo -g $url` Is even simpler. -- Gilles. pgpbhFxbUoSjt.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Thu, Apr 09, 2015 at 01:08:21PM +0200, wm4 wrote: > On Thu, 9 Apr 2015 00:02:50 +0200 > Lukasz Marek wrote: > > > W dniu środa, 8 kwietnia 2015 Gilles Chanteperdrix < > > gilles.chanteperd...@xenomai.org> > > > > And forgot to add my apologies for these bad patches and the noise > > > that followed. > > > > > > > No need for that. > > > > Your demuxer seems to use public API only so you can try to move this > > implementation to your project and register this demuxer from your > > application. > > I'm pretty sure implementing a demuxer requires using private API. > > What's more, this stuff doesn't make sense as a demuxer. I am not sure whether it makes sense, but implementing a fallback in the application which spawns youtube-dl then retry when avformat_open_input fails for an http/https URL is in fact simpler than implementing a demuxer. And it avoids the noticeable added latency which results from running youtube-dl (or libquvi for that matter) for every http URL directly supported by ffmpeg. -- Gilles. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat: add youtube-dl based demuxer
On Thu, Apr 09, 2015 at 03:37:30PM +0200, Nicolas George wrote:
> Le decadi 20 germinal, an CCXXIII, Gilles Chanteperdrix a écrit :
> > Well, in that case,
> > ffmpeg -i `youtube-dl -f bestvideo -g $url`
>
> That could work too, but you still fail to acknowledge that -g is frequently
> not enough.
And -J is to much: it gives you all the available videos and forces
you to reimplement youtube-dl -f option to choose only one from the
list. In what case is -g not sufficient, for handling play lists ?
--
Gilles.
pgpzg4vkDQlzD.pgp
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
