Re: [FFmpeg-devel] [PATCH 2/5] avcodec/vc1: fix mquant calculation for interlace field pictures

2018-05-20 Thread Carl Eugen Hoyos 
2018-05-20 11:00 GMT+02:00, Jerome Borsboom :

>>> for patch 1 you list a file that it improves.
>>
>> The file - afaict - is related to the patchset as a whole.
>> The current version of the patchset does not fix the
>> file though, so I believe we should wait for a new
>> version of the patchset.

> The issue that remains is not related to this patch set. In
> my view, there is no need to wait for a new patch set.

Thank you for clarifying this!

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v4 1/3] avformat: add fields to AVProgram/AVStream for PMT change tracking

2018-05-20 Thread Carl Eugen Hoyos 
2018-05-19 23:48 GMT+02:00, James Almer :
> On 5/19/2018 6:31 PM, Michael Niedermayer wrote:
>> On Fri, May 18, 2018 at 11:15:04AM -0700, Aman Gupta wrote:
>>> From: Aman Gupta 
>>>
>>> These fields will allow the mpegts demuxer to expose details about
>>> the PMT/program which created the AVProgram and its AVStreams.
>>>
>>> In mpegts, a PMT which advertises streams has a version number
>>> which can be incremented at any time. When the version changes,
>>> the pids which correspond to each of it's streams can also change.
>>>
>>> Since ffmpeg creates a new AVStream per pid by default, an API user
>>> needs the ability to (a) detect when the PMT changed, and (b) tell
>>> which AVStream were added to replace earlier streams.
>>>
>>> This has been a long-standing issue with ffmpeg's handling of mpegts
>>> streams with PMT changes, and I found two related patches in the wild
>>> that attempt to solve the same problem.
>>>
>>> The first is in MythTV's ffmpeg fork, where they added a
>>> void (*streams_changed)(void*); to AVFormatContext and call it from
>>> their fork of the mpegts demuxer whenever the PMT changes.
>>>
>>> The second was proposed by XBMC in
>>> https://ffmpeg.org/pipermail/ffmpeg-devel/2012-December/135036.html,
>>> where they created a new AVMEDIA_TYPE_DATA stream with id=0 and
>>> attempted to send packets to it whenever the PMT changed.
>>>
>>> Signed-off-by: Aman Gupta 
>>> ---
>>>  doc/APIchanges | 4 
>>>  libavformat/avformat.h | 8 
>>>  libavformat/utils.c| 1 +
>>>  libavformat/version.h  | 2 +-
>>>  4 files changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/doc/APIchanges b/doc/APIchanges
>>> index befa58c84a..a592073ca5 100644
>>> --- a/doc/APIchanges
>>> +++ b/doc/APIchanges
>>> @@ -15,6 +15,10 @@ libavutil: 2017-10-21
>>>
>>>  API changes, most recent first:
>>>
>>> +2018-05-xx - xx - lavf 58.15.100 - avformat.h
>>> +  Add pmt_version field to AVProgram
>>> +  Add program_num, pmt_version, pmt_stream_idx to AVStream
>>> +
>>>  2018-05-xx - xx - lavf 58.14.100 - avformat.h
>>>Add AV_DISPOSITION_STILL_IMAGE
>>>
>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>> index 6dce88fad5..ade918f99c 100644
>>> --- a/libavformat/avformat.h
>>> +++ b/libavformat/avformat.h
>>> @@ -1103,6 +1103,13 @@ typedef struct AVStream {
>>>   */
>>>  int stream_identifier;
>>>
>>> +/**
>>> + * Details of the MPEG-TS program which created this stream.
>>> + */
>>> +int program_num;
>>> +int pmt_version;
>>> +int pmt_stream_idx;
>>> +
>>>  int64_t interleaver_chunk_size;
>>>  int64_t interleaver_chunk_duration;
>>>
>>
>> These are added below the "All fields below this line are not part of the
>> public API."
>> This contradicts the addition to APIChanges
>
> If these don't need to be accessed by the API user then I'd rather keep
> them here than moving them to the public section. Just remove the
> APIChanges entry.
> Same thing with the AVStream pmt_version field. If direct access is not
> needed for it, then it should be moved below the public notice.
>
> These fields seem pretty specific to one demuxer so ideally they
> shouldn't be public in case changes to them and the implementation are
> ever needed.

Reading the original submission above, isn't the whole point of the
patch to add public symbols to help downstream with real-world
issues?

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 5/5] avcodec/vc1: store zero MVs for all blocks in a MB

2018-05-20 Thread Jerome Borsboom 
>> Direct prediction for interlace frame B pictures references the mv in the
>> second block in an MB in the backward reference frame for the twomv case.
>> When the backward reference frame is an I frame, this value may be unset.
>> 
>> Signed-off-by: Jerome Borsboom 
>> ---
>>  libavcodec/vc1_block.c | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavcodec/vc1_block.c b/libavcodec/vc1_block.c
>> index 74935ec9e9..9c170a1e3c 100644
>> --- a/libavcodec/vc1_block.c
>> +++ b/libavcodec/vc1_block.c
>> @@ -2678,8 +2678,10 @@ static void vc1_decode_i_blocks_adv(VC1Context *v)
>>  s->bdsp.clear_blocks(block[0]);
>>  mb_pos = s->mb_x + s->mb_y * s->mb_stride;
>>  s->current_picture.mb_type[mb_pos + v->mb_off]  
>>= MB_TYPE_INTRA;
>> -s->current_picture.motion_val[1][s->block_index[0] + 
>> v->blocks_off][0] = 0;
>> -s->current_picture.motion_val[1][s->block_index[0] + 
>> v->blocks_off][1] = 0;
>> +for (int i = 0; i < 4; i++) {
>> +s->current_picture.motion_val[1][s->block_index[i] + 
>> v->blocks_off][0] = 0;
>> +s->current_picture.motion_val[1][s->block_index[i] + 
>> v->blocks_off][1] = 0;
>> +}
> 
> see AV_ZERO*

This style of setting motion_val to zero is used all over the VC-1
decoder. vc1_decode_p_mb_intfr uses the exact same code. Changing to
AV_ZERO may certainly a good point for improvement, however, for the
sake of consistency the proposed code might be preferable.

Regards,
Jerome
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] avcodec/vc1: FIELDTX is only coded raw in interlaced frame I pictures

2018-05-20 Thread Michael Niedermayer 
On Sun, May 20, 2018 at 10:56:19AM +0200, Jerome Borsboom wrote:
> >>  libavcodec/vc1_block.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/libavcodec/vc1_block.c b/libavcodec/vc1_block.c
> >> index f59c440943..daf30fdbfe 100644
> >> --- a/libavcodec/vc1_block.c
> >> +++ b/libavcodec/vc1_block.c
> >> @@ -2680,7 +2680,7 @@ static void vc1_decode_i_blocks_adv(VC1Context *v)
> >>  s->current_picture.motion_val[1][s->block_index[0] + 
> >> v->blocks_off][1] = 0;
> >>  
> >>  // do actual MB decoding and displaying
> >> -if (v->fieldtx_is_raw)
> >> +if (v->fcm == ILACE_FRAME && v->fieldtx_is_raw)
> >>  v->fieldtx_plane[mb_pos] = get_bits1(>s.gb);
> > 
> > fieldtx_is_raw is only set when fcm == ILACE_FRAME
> > I suspect the intend was it is unset otherwise. This would avoid the extra
> > check
> 
> I think this may be a design decision. You can either set the decoding
> context, 'v' in this case, to a sane default or only use syntax elements
> where appropriate. The current state of the bitstream decoder for VC-1
> is not very clean in this regard. But it does certainly not reset the
> decoding context for each frame and even depends on this behaviour for
> at least one case.
> 
> While I think it may be better to reset the decoding context to sane
> defaults for each frame for the applicable variables, for now, I would
> like to propose to leave the bitstream decoder as is and use this patch.
> Currently, I am focusing on compliance to the VC-1 spec and a cleanup of
> the bitstream decoder may be something down the road.

you are the one working on this so its clearly up to you to decide these
things.
Personally i would not leave fieldtx_plane at non zero value if that mode
is not enabled


> 
> A trac issue might be appropriate to remember this issue, 

yes


> although the
> decoder in general could use a good cleanup.

yes

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] avcodec/vc1: FIELDTX is only coded raw in interlaced frame I pictures

2018-05-20 Thread Jerome Borsboom 
>>  libavcodec/vc1_block.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/libavcodec/vc1_block.c b/libavcodec/vc1_block.c
>> index f59c440943..daf30fdbfe 100644
>> --- a/libavcodec/vc1_block.c
>> +++ b/libavcodec/vc1_block.c
>> @@ -2680,7 +2680,7 @@ static void vc1_decode_i_blocks_adv(VC1Context *v)
>>  s->current_picture.motion_val[1][s->block_index[0] + 
>> v->blocks_off][1] = 0;
>>  
>>  // do actual MB decoding and displaying
>> -if (v->fieldtx_is_raw)
>> +if (v->fcm == ILACE_FRAME && v->fieldtx_is_raw)
>>  v->fieldtx_plane[mb_pos] = get_bits1(>s.gb);
> 
> fieldtx_is_raw is only set when fcm == ILACE_FRAME
> I suspect the intend was it is unset otherwise. This would avoid the extra
> check

I think this may be a design decision. You can either set the decoding
context, 'v' in this case, to a sane default or only use syntax elements
where appropriate. The current state of the bitstream decoder for VC-1
is not very clean in this regard. But it does certainly not reset the
decoding context for each frame and even depends on this behaviour for
at least one case.

While I think it may be better to reset the decoding context to sane
defaults for each frame for the applicable variables, for now, I would
like to propose to leave the bitstream decoder as is and use this patch.
Currently, I am focusing on compliance to the VC-1 spec and a cleanup of
the bitstream decoder may be something down the road.

A trac issue might be appropriate to remember this issue, although the
decoder in general could use a good cleanup.

Regards,
Jerome
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v4 1/3] avformat: add fields to AVProgram/AVStream for PMT change tracking

2018-05-20 Thread Jan Ekström 
On Sun, May 20, 2018 at 1:54 PM, Carl Eugen Hoyos  wrote:
>
> Reading the original submission above, isn't the whole point of the
> patch to add public symbols to help downstream with real-world
> issues?
>
> Carl Eugen

Yes, currently the API user has no visibility over how the programs
are being updated. Two alternative takes on this were listed in the
commit message, one being a call-back and another being a custom data
packet from stream id 0.

So yes, the pmt_version is most definitely meant to be publicly
utilized, with it the proper utilization would go as follows:
1) API client picks the program(s) it cares about, and gets the
AVProgram(s)' pointers and notes the pmt_version when it started
reading.
2) After each AVPacket read, it checks if the pmt_version was updated.
If it was, it iterates over the streams currently registered in the
AVProgram(s) it cares about.
3) If any streams got dropped, those should be no longer cared about
by the API client. If any streams got added, they should be checked by
the API client if it is interested in them.

This is an upgrade on the previous state of affairs which would have
required the following:
1) API client picks the program(s) it cares about, and gets the
AVProgram(s)' pointers.
2) After each AVPacket read, it iterates over the streams currently
registered in the AVProgram(s) it cares about.
3) If any streams got dropped, those should be no longer cared about
by the API client. If any streams got added, they should be checked by
the API client if it is interested in them.

It is not perfect, but still an upgrade, since you can just check an
integer now, instead of doing the full iteration every time you read
an AVPacket.


Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/5] avcodec/vc1: fix mquant calculation for interlace field pictures

2018-05-20 Thread Jerome Borsboom 
>> On Fri, May 18, 2018 at 05:06:23PM +0200, Jerome Borsboom wrote:
>>> For interlace field pictures s->mb_height indicates the height of the
>>> full
>>> picture in MBs, i.e. the two fields combined. A single field is half this
>>> size. When calculating mquant for interlace field pictures, the bottom
>>> edge
>>> is the last MB row of the field.
>>>
>>> Signed-off-by: Jerome Borsboom 
>>> ---
>>>  libavcodec/vc1_block.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> for patch 1 you list a file that it improves.
> 
> The file - afaict - is related to the patchset as a whole.
> The current version of the patchset does not fix the
> file though, so I believe we should wait for a new
> version of the patchset.
> 
> The sample is >20MB, most of the reference files are
> very small, so perhaps we can avoid using this one.
> 
> Carl Eugen

The issue that remains is not related to this patch set. In my view,
there is no need to wait for a new patch set. I will just submit a new
patch for that issue.

Regards,
Jerome
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v4 1/3] avformat: add fields to AVProgram/AVStream for PMT change tracking

2018-05-20 Thread Jan Ekström 
On Sun, May 20, 2018 at 2:06 PM, Jan Ekström  wrote:
> On Sun, May 20, 2018 at 1:54 PM, Carl Eugen Hoyos  wrote:
>>
>> Reading the original submission above, isn't the whole point of the
>> patch to add public symbols to help downstream with real-world
>> issues?
>>
>> Carl Eugen
>
> Yes, currently the API user has no visibility over how the programs
> are being updated. Two alternative takes on this were listed in the
> commit message, one being a call-back and another being a custom data
> packet from stream id 0.
>
> So yes, the pmt_version is most definitely meant to be publicly
> utilized, with it the proper utilization would go as follows:
> 1) API client picks the program(s) it cares about, and gets the
> AVProgram(s)' pointers and notes the pmt_version when it started
> reading.
> 2) After each AVPacket read, it checks if the pmt_version was updated.
> If it was, it iterates over the streams currently registered in the
> AVProgram(s) it cares about.
> 3) If any streams got dropped, those should be no longer cared about
> by the API client. If any streams got added, they should be checked by
> the API client if it is interested in them.
>
> This is an upgrade on the previous state of affairs which would have
> required the following:
> 1) API client picks the program(s) it cares about, and gets the
> AVProgram(s)' pointers.
> 2) After each AVPacket read, it iterates over the streams currently
> registered in the AVProgram(s) it cares about.
> 3) If any streams got dropped, those should be no longer cared about
> by the API client. If any streams got added, they should be checked by
> the API client if it is interested in them.
>
> It is not perfect, but still an upgrade, since you can just check an
> integer now, instead of doing the full iteration every time you read
> an AVPacket.
>
>
> Best regards,
> Jan

Oh, and I'm not sure if lavf's internal reordering of packets could
make it happen that you receive the program update, and then still
receive one or more packets from the previous stream. In that case you
might also incorporate some check that the flush and reset of previous
decoder only happens after you get your first new packet for that new
stream.

Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 1/5] avcodec/vc1: FIELDTX is only present in interlaced frame I/BI pictures

2018-05-20 Thread Jerome Borsboom 
If v->fieldtx_is_raw is not reset to zero, it may spill over from a previous
interlaced frame I/BI picture.

Signed-off-by: Jerome Borsboom 
---
This may address the concerns. Will make a mental note to clean up the parser
at a later time.

Thank you for the review.

 libavcodec/vc1.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/vc1.c b/libavcodec/vc1.c
index 949fec6bee..98b24e8e57 100644
--- a/libavcodec/vc1.c
+++ b/libavcodec/vc1.c
@@ -1010,7 +1010,8 @@ int ff_vc1_parse_frame_header_adv(VC1Context *v, 
GetBitContext* gb)
 return -1;
 av_log(v->s.avctx, AV_LOG_DEBUG, "FIELDTX plane encoding: "
"Imode: %i, Invert: %i\n", status>>1, status&1);
-}
+} else
+v->fieldtx_is_raw = 0;
 status = bitplane_decoding(v->acpred_plane, >acpred_is_raw, v);
 if (status < 0)
 return -1;
-- 
2.13.6


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] HLS Questions

2018-05-20 Thread Ronak Patel 
No I’ll give it a try and let you know. 

Since this is just audio (no keyframes) I wouldn’t have thought this would have 
mattered.

Sent from my iPhone

> On May 7, 2018, at 3:52 PM, Aman Gupta  wrote:
> 
> On Wed, May 2, 2018 at 7:59 AM, Ronak Patel <
> ronak2121-at-yahoo@ffmpeg.org> wrote:
> 
>> Hi all,
>> 
>> So I’ve noticed that ffmpeg does not always properly follow the number we
>> specify for hls_time when generating hls content.
>> 
>> For example, if we have an MP4/AAC file at 44.1kHz sampling rate, we would
>> expect that specifying 9.75238095238095 (420 frames) would return a
>> manifest with the same exact amount of frames throughout.
>> Instead, we see the AAC encoder vary the amount of frames without any real
>> explanation.
>> 
> 
> Did you try -hls_flags split_by_time ?
> 
> Aman
> 
> 
>> 
>> When we run the following command:
>> 
>> /home/ronakp/bin/ffmpeg -i "${FILE}.mp4" -codec copy -hls_time
>> 9.75238095238095 -hls_segment_type fmp4 -hls_flags single_file+append_list
>> -hls_playlist_type vod "${FILE}_10sec_v7.m3u8"
>> 
>> We get a manifest that looks like so:
>> 
>> #EXTM3U
>> #EXT-X-VERSION:7
>> #EXT-X-TARGETDURATION:10
>> #EXT-X-MEDIA-SEQUENCE:0
>> #EXT-X-PLAYLIST-TYPE:VOD
>> #EXT-X-MAP:URI="bk_acx0_006556_22_32_10sec_v7.m4s",BYTERANGE="738@0"
>> #EXT-X-DISCONTINUITY
>> #EXTINF:9.798821,
>> #EXT-X-BYTERANGE:38733@738
>> bk_acx0_006556_22_32_10sec_v7.m4s
>> #EXTINF:9.752381,
>> #EXT-X-BYTERANGE:38531@39471
>> bk_acx0_006556_22_32_10sec_v7.m4s
>> #EXTINF:9.752381,
>> #EXT-X-BYTERANGE:38616@78002
>> bk_acx0_006556_22_32_10sec_v7.m4s
>> #EXTINF:9.752381,
>> #EXT-X-BYTERANGE:38545@116618
>> bk_acx0_006556_22_32_10sec_v7.m4s
>> 
>> Notice the first segment there contains 2 more AAC frames...
>> 
>> Now, when we run the following command:
>> 
>> /home/ronakp/bin/ffmpeg -i "${FILE}.mp4" -codec copy -hls_time
>> 0.975238095238095 -hls_segment_type fmp4 -hls_flags single_file+append_list
>> -hls_playlist_type vod "${FILE}_10sec_v7.m3u8"
>> 
>> #EXTM3U
>> #EXT-X-VERSION:7
>> #EXT-X-TARGETDURATION:1
>> #EXT-X-MEDIA-SEQUENCE:0
>> #EXT-X-PLAYLIST-TYPE:VOD
>> #EXT-X-MAP:URI="bk_reco_004353_22_32_1sec_v7.m4s",BYTERANGE="738@0"
>> #EXT-X-DISCONTINUITY
>> #EXTINF:0.975238,
>> #EXT-X-BYTERANGE:4110@738
>> bk_reco_004353_22_32_1sec_v7.m4s
>> #EXTINF:0.975238,
>> #EXT-X-BYTERANGE:3897@4848
>> bk_reco_004353_22_32_1sec_v7.m4s
>> #EXTINF:0.975238,
>> #EXT-X-BYTERANGE:4236@8745
>> bk_reco_004353_22_32_1sec_v7.m4s
>> #EXTINF:0.975238,
>> 
>> It seems to have the correct amount of frames everywhere.
>> 
>> Even more bizarre is if we run this command:
>> 
>> /home/ronakp/bin/ffmpeg -i "../${FILE}.mp4" -codec copy -hls_time
>> 9.75238095238095 -hls_segment_filename 'file%03d.ts' -hls_segment_type
>> mpegts -hls_playlist_type vod "${FILE}_10sec_v3.m3u8"
>> 
>> The manifest looks like the following:
>> 
>> #EXTM3U
>> #EXT-X-VERSION:3
>> #EXT-X-TARGETDURATION:10
>> #EXT-X-MEDIA-SEQUENCE:0
>> #EXT-X-PLAYLIST-TYPE:VOD
>> #EXTINF:9.799778,
>> file000.ts
>> #EXTINF:9.706889,
>> file001.ts
>> #EXTINF:9.75,
>> file002.ts
>> #EXTINF:9.799778,
>> file003.ts
>> #EXTINF:9.75,
>> file004.ts
>> #EXTINF:9.706889,
>> file005.ts
>> #EXTINF:9.799778,
>> file006.ts
>> 
>> The ts segments do not all have the same size, and seems like ffmpeg is
>> unable to properly honor the supplied time.
>> 
>> And finally, when I run the following command:
>> 
>> /home/ronakp/bin/ffmpeg -i "${FILE}.mp4" -codec copy -f dash
>> -single_file_name "${FILE}_1sec.m4s" -min_seg_duration 975238.095238095
>> -hls_playlist 1 "${FILE}_1sec.mpd"
>> 
>> I get a manifest that looks like:
>> 
>> #EXTM3U
>> #EXT-X-VERSION:6
>> #EXT-X-TARGETDURATION:10
>> #EXT-X-MEDIA-SEQUENCE:1
>> #EXT-X-MAP:URI="bk_reco_004353_22_32_10sec.m4s",BYTERANGE="738@0"
>> #EXTINF:9.798821,
>> #EXT-X-BYTERANGE:38996@738
>> bk_reco_004353_22_32_10sec.m4s
>> #EXTINF:9.798821,
>> #EXT-X-BYTERANGE:38754@39734
>> bk_reco_004353_22_32_10sec.m4s
>> #EXTINF:9.798821,
>> #EXT-X-BYTERANGE:38765@78488
>> bk_reco_004353_22_32_10sec.m4s
>> #EXTINF:9.798821,
>> #EXT-X-BYTERANGE:38583@117253
>> bk_reco_004353_22_32_10sec.m4s
>> #EXTINF:9.798821,
>> #EXT-X-BYTERANGE:38752@155836
>> bk_reco_004353_22_32_10sec.m4s
>> 
>> Where every fragment has 1 extra frame in it.
>> 
>> Can someone please help get this fixed? The fragmentation should have been
>> uniform throughout and exactly the same no matter which options I specify
>> here.
>> 
>> For reference, I'm using the following versions of ffmpeg:
>> 
>> ffmpeg version N-90757-g2fc12f4 Copyright (c) 2000-2018 the FFmpeg
>> developers
>>  built with gcc 4.4.6 (GCC) 20110731 (Red Hat 4.4.6-3)
>>  configuration: --prefix=/home/ronakp/ffmpeg_build
>> --pkg-config-flags=--static 
>> --extra-cflags=-I/home/ronakp/ffmpeg_build/include
>> --extra-ldflags=-L/home/ronakp/ffmpeg_build/lib --extra-libs=-lpthread
>> --extra-libs=-lm --bindir=/home/ronakp/bin --enable-gpl --enable-avisynth
>> --enable-libfdk_aac --enable-libmp3lame

Re: [FFmpeg-devel] [PATCH 2/5] avcodec/vc1: fix mquant calculation for interlace field pictures

2018-05-20 Thread Carl Eugen Hoyos 
2018-05-20 0:01 GMT+02:00, Michael Niedermayer :
> On Fri, May 18, 2018 at 05:06:23PM +0200, Jerome Borsboom wrote:
>> For interlace field pictures s->mb_height indicates the height of the
>> full
>> picture in MBs, i.e. the two fields combined. A single field is half this
>> size. When calculating mquant for interlace field pictures, the bottom
>> edge
>> is the last MB row of the field.
>>
>> Signed-off-by: Jerome Borsboom 
>> ---
>>  libavcodec/vc1_block.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> for patch 1 you list a file that it improves.

The file - afaict - is related to the patchset as a whole.
The current version of the patchset does not fix the
file though, so I believe we should wait for a new
version of the patchset.

The sample is >20MB, most of the reference files are
very small, so perhaps we can avoid using this one.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avutil/error: document av_err2str

2018-05-20 Thread Tomas Härdin 
tor 2018-05-17 klockan 12:19 +0200 skrev Tobias Rapp:
> On 17.05.2018 11:24, Zhao Zhili wrote:
> > ---
> >   libavutil/error.h | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/libavutil/error.h b/libavutil/error.h
> > index 71df4da..b357bfa 100644
> > --- a/libavutil/error.h
> > +++ b/libavutil/error.h
> > @@ -115,6 +115,9 @@ static inline char *av_make_error_string(char *errbuf, 
> > size_t errbuf_size, int e
> >   /**
> >    * Convenience macro, the return value should be used only directly in
> >    * function arguments but never stand-alone.
> > + *
> > + * @warning The macro uses compound literal which was introduced in C99. 
> > It may
> > + *  not work in C++.
> >    */
> >   #define av_err2str(errnum) \
> >   av_make_error_string((char[AV_ERROR_MAX_STRING_SIZE]){0}, 
> > AV_ERROR_MAX_STRING_SIZE, errnum)
> > 
> 
> If not using the macro is enough to silence affected C++ compilers, I 
> prefer this patch over the other one that hides the macro with ifdefs.

I agree

/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 5/5] avcodec/vc1: store zero MVs for all blocks in a MB

2018-05-20 Thread Carl Eugen Hoyos 
2018-05-20 3:45 GMT+02:00, James Almer :

> I see no emms in either ff_clear_blocks_mmx() or anywhere in
> vc1_block.c, so either nobody tried this decoder on machines
> without sse

I do have access to an SSE-only system, I doubt FFmpeg was
used or tested on MMX-only systems in a very, very long time.

musl doesn't work regardless of this particular possible issue,
so it cannot be used to test it.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 3/8] ffserver.c: rename ReadInfo.in_filename to ReadInfo.in_uri

2018-05-20 Thread Stephan Holljes 
Signed-off-by: Stephan Holljes 
---
 ffserver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ffserver.c b/ffserver.c
index bd7c694..44fc263 100644
--- a/ffserver.c
+++ b/ffserver.c
@@ -45,7 +45,7 @@
 struct ReadInfo {
 struct PublisherContext *pub;
 AVFormatContext *ifmt_ctx;
-char *in_filename;
+char *input_uri;
 };
 
 struct WriteInfo {
-- 
2.16.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v4 1/3] avformat: add fields to AVProgram/AVStream for PMT change tracking

2018-05-20 Thread James Almer 
On 5/20/2018 3:37 PM, Aman Gupta wrote:
> On Sat, May 19, 2018 at 2:56 PM James Almer  wrote:
> 
>> On 5/19/2018 6:31 PM, Michael Niedermayer wrote:
>>> On Fri, May 18, 2018 at 11:15:04AM -0700, Aman Gupta wrote:
 From: Aman Gupta 

 These fields will allow the mpegts demuxer to expose details about
 the PMT/program which created the AVProgram and its AVStreams.

 In mpegts, a PMT which advertises streams has a version number
 which can be incremented at any time. When the version changes,
 the pids which correspond to each of it's streams can also change.

 Since ffmpeg creates a new AVStream per pid by default, an API user
 needs the ability to (a) detect when the PMT changed, and (b) tell
 which AVStream were added to replace earlier streams.

 This has been a long-standing issue with ffmpeg's handling of mpegts
 streams with PMT changes, and I found two related patches in the wild
 that attempt to solve the same problem.

 The first is in MythTV's ffmpeg fork, where they added a
 void (*streams_changed)(void*); to AVFormatContext and call it from
 their fork of the mpegts demuxer whenever the PMT changes.

 The second was proposed by XBMC in
 https://ffmpeg.org/pipermail/ffmpeg-devel/2012-December/135036.html,
 where they created a new AVMEDIA_TYPE_DATA stream with id=0 and
 attempted to send packets to it whenever the PMT changed.

 Signed-off-by: Aman Gupta 
 ---
  doc/APIchanges | 4 
  libavformat/avformat.h | 8 
  libavformat/utils.c| 1 +
  libavformat/version.h  | 2 +-
  4 files changed, 14 insertions(+), 1 deletion(-)

 diff --git a/doc/APIchanges b/doc/APIchanges
 index befa58c84a..a592073ca5 100644
 --- a/doc/APIchanges
 +++ b/doc/APIchanges
 @@ -15,6 +15,10 @@ libavutil: 2017-10-21

  API changes, most recent first:

 +2018-05-xx - xx - lavf 58.15.100 - avformat.h
 +  Add pmt_version field to AVProgram
 +  Add program_num, pmt_version, pmt_stream_idx to AVStream
 +
  2018-05-xx - xx - lavf 58.14.100 - avformat.h
Add AV_DISPOSITION_STILL_IMAGE

 diff --git a/libavformat/avformat.h b/libavformat/avformat.h
 index 6dce88fad5..ade918f99c 100644
 --- a/libavformat/avformat.h
 +++ b/libavformat/avformat.h
 @@ -1103,6 +1103,13 @@ typedef struct AVStream {
   */
  int stream_identifier;

 +/**
 + * Details of the MPEG-TS program which created this stream.
 + */
 +int program_num;
 +int pmt_version;
 +int pmt_stream_idx;
 +
  int64_t interleaver_chunk_size;
  int64_t interleaver_chunk_duration;

>>>
>>> These are added below the "All fields below this line are not part of
>> the public API."
>>> This contradicts the addition to APIChanges
>>
>> If these don't need to be accessed by the API user then I'd rather keep
>> them here than moving them to the public section. Just remove the
>> APIChanges entry.
>> Same thing with the AVStream pmt_version field. If direct access is not
>> needed for it, then it should be moved below the public notice.
>>
>> These fields seem pretty specific to one demuxer so ideally they
>> shouldn't be public in case changes to them and the implementation are
>> ever needed.
> 
> 
> Yes they are specific to mpegts, and also may need to change in the future.
> 
> These were intended to be used in conjunction with the existing
> AVStream.stream_identifier, which is also mpegts-specific. It appears in
> the private section, even though the field is only written to from mpegts.c
> and never used anywhere else in avformat. Clearly it was added to be used
> by API users, but it too lives in the private section.
> 
> It seems like the best course forward is to remove APIchanges entries. The
> fields are there if a user really needs to access them, but they are
> considered private which means we can change them later if need be to
> evolve support. In this case, should I revert the version bump as well? Or
> is that still required since the ABI changed (even though it was in the
> private section).

Since they are in the private section then yes, remove the relevant
APIChanges entry, but don't revert the version bump (It's never a clean
thing to do and it's best to avoid it).
For that matter, new fields added to the private section of these
structs don't change the ABI since their size isn't part of it. They can
be removed and changed at any time without a major bump, which is why
API users are not meant to ever access them directly.

And again, if AVStream.pmt_version is also meant to be private and
change in the future, then please move it to the private section.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org

Re: [FFmpeg-devel] [PATCH 6/8] ffserver.c: Add config file reading

2018-05-20 Thread Michael Niedermayer 
On Sun, May 20, 2018 at 08:54:02PM +0200, Stephan Holljes wrote:
> Signed-off-by: Stephan Holljes 
> ---
>  ffserver.c | 248 
> ++---
>  1 file changed, 172 insertions(+), 76 deletions(-)
> 
> diff --git a/ffserver.c b/ffserver.c
> index 44fc263..3d842d8 100644
> --- a/ffserver.c
> +++ b/ffserver.c
> @@ -38,6 +38,7 @@
>  #include "segment.h"
>  #include "publisher.h"
>  #include "httpd.h"
> +#include "configreader.h"
>  
>  #define BUFFER_SECS 30
>  #define LISTEN_TIMEOUT_MSEC 1000
> @@ -54,9 +55,11 @@ struct WriteInfo {
>  };
>  
>  struct AcceptInfo {
> -struct PublisherContext *pub;
> +struct PublisherContext **pubs;
>  struct HTTPDInterface *httpd;
> -AVFormatContext *ifmt_ctx;
> +AVFormatContext **ifmt_ctxs;
> +struct HTTPDConfig *config;
> +int nb_pub; /* number of publishers (streams) equal to number of 
> ifmt_ctx */
>  };
>  
>  
> @@ -287,52 +290,77 @@ void *accept_thread(void *arg)
>  {
>  struct AcceptInfo *info = (struct AcceptInfo*) arg;
>  struct FFServerInfo *ffinfo = NULL;
> +struct PublisherContext *pub;
>  char status[4096];
> +char *stream_name;
>  struct HTTPClient *client = NULL;
>  void *server = NULL;
>  AVIOContext *client_ctx = NULL;
>  AVFormatContext *ofmt_ctx = NULL;
> +AVFormatContext *ifmt_ctx;
>  unsigned char *avio_buffer;
>  AVOutputFormat *ofmt;
>  AVDictionary *mkvopts = NULL;
>  AVStream *in_stream, *out_stream;
>  int ret, i, reply_code;
> -struct HTTPDConfig config = {
> -.bind_address = "0",
> -.port = 8080,
> -.accept_timeout = LISTEN_TIMEOUT_MSEC,
> -};
> -
> -info->httpd->init(, config);
> -
> -
> +int shutdown;
> +struct HTTPDConfig *config = info->config;
> +
> +info->httpd->init(, *config);
> +
>  for (;;) {
> -if (info->pub->shutdown)
> +shutdown = 1;
> +for (i = 0; i < config->nb_streams; i++) {
> +if (info->pubs[i] && !info->pubs[i]->shutdown)
> +shutdown = 0;
> +}
> +if (shutdown)
>  break;
> -publisher_gen_status_json(info->pub, status);
> -av_log(server, AV_LOG_INFO, status);
> +for (i = 0; i < config->nb_streams; i++) {
> +publisher_gen_status_json(info->pubs[i], status);
> +av_log(server, AV_LOG_INFO, status);
> +}
>  client = NULL;
>  av_log(server, AV_LOG_DEBUG, "Accepting new clients.\n");
>  reply_code = 200;
> -if (publisher_reserve_client(info->pub)) {
> -av_log(client, AV_LOG_WARNING, "No more client slots free, 
> Returning 503.\n");
> -reply_code = 503;
> -}
> -
> +
>  if ((ret = info->httpd->accept(server, , reply_code)) < 0) {
>  if (ret == HTTPD_LISTEN_TIMEOUT) {
> -publisher_cancel_reserve(info->pub);
>  continue;
>  } else if (ret == HTTPD_CLIENT_ERROR) {
>  info->httpd->close(server, client);
>  }
>  av_log(server, AV_LOG_WARNING, "Error during accept, 
> retrying.\n");
> -publisher_cancel_reserve(info->pub);
>  continue;
>  }
> -
> +
> +pub = NULL;
> +ifmt_ctx = NULL;
> +for (i = 0; i < config->nb_streams; i++) {
> +stream_name = info->pubs[i]->stream_name;
> +//   skip leading '/'  ---v
> +if(!strncmp(client->resource + 1, stream_name, 
> strlen(stream_name))) {
> +pub = info->pubs[i];
> +ifmt_ctx = info->ifmt_ctxs[i];
> +break;
> +}
> +}
> +
> +if (!pub || !ifmt_ctx) {
> +av_log(client_ctx, AV_LOG_WARNING, "No suitable publisher found 
> for resource: %s.\n",
> +client->resource ? 
> client->resource : "(null)");
> +reply_code = 404;
> +}
> +
> +
> +if (pub && ifmt_ctx && publisher_reserve_client(pub)) {
> +av_log(client_ctx, AV_LOG_WARNING, "No more client slots free, 
> Returning 503.\n");
> +reply_code = 503;
> +}
> +
>  if (reply_code != 200) {
> -publisher_cancel_reserve(info->pub);
> +if (pub && ifmt_ctx)
> +publisher_cancel_reserve(pub);
>  info->httpd->close(server, client);
>  continue;
>  }
> @@ -345,7 +373,7 @@ void *accept_thread(void *arg)
>  client_ctx = avio_alloc_context(avio_buffer, AV_BUFSIZE, 1, ffinfo, 
> NULL, _write, NULL);
>  if (!client_ctx) {
>  av_log(client, AV_LOG_ERROR, "Could not allocate output format 
> context.\n");
> -publisher_cancel_reserve(info->pub);
> +publisher_cancel_reserve(pub);
>  info->httpd->close(server,

Re: [FFmpeg-devel] [PATCH v4 1/3] avformat: add fields to AVProgram/AVStream for PMT change tracking

2018-05-20 Thread Aman Gupta 
On Sat, May 19, 2018 at 2:56 PM James Almer  wrote:

> On 5/19/2018 6:31 PM, Michael Niedermayer wrote:
> > On Fri, May 18, 2018 at 11:15:04AM -0700, Aman Gupta wrote:
> >> From: Aman Gupta 
> >>
> >> These fields will allow the mpegts demuxer to expose details about
> >> the PMT/program which created the AVProgram and its AVStreams.
> >>
> >> In mpegts, a PMT which advertises streams has a version number
> >> which can be incremented at any time. When the version changes,
> >> the pids which correspond to each of it's streams can also change.
> >>
> >> Since ffmpeg creates a new AVStream per pid by default, an API user
> >> needs the ability to (a) detect when the PMT changed, and (b) tell
> >> which AVStream were added to replace earlier streams.
> >>
> >> This has been a long-standing issue with ffmpeg's handling of mpegts
> >> streams with PMT changes, and I found two related patches in the wild
> >> that attempt to solve the same problem.
> >>
> >> The first is in MythTV's ffmpeg fork, where they added a
> >> void (*streams_changed)(void*); to AVFormatContext and call it from
> >> their fork of the mpegts demuxer whenever the PMT changes.
> >>
> >> The second was proposed by XBMC in
> >> https://ffmpeg.org/pipermail/ffmpeg-devel/2012-December/135036.html,
> >> where they created a new AVMEDIA_TYPE_DATA stream with id=0 and
> >> attempted to send packets to it whenever the PMT changed.
> >>
> >> Signed-off-by: Aman Gupta 
> >> ---
> >>  doc/APIchanges | 4 
> >>  libavformat/avformat.h | 8 
> >>  libavformat/utils.c| 1 +
> >>  libavformat/version.h  | 2 +-
> >>  4 files changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/doc/APIchanges b/doc/APIchanges
> >> index befa58c84a..a592073ca5 100644
> >> --- a/doc/APIchanges
> >> +++ b/doc/APIchanges
> >> @@ -15,6 +15,10 @@ libavutil: 2017-10-21
> >>
> >>  API changes, most recent first:
> >>
> >> +2018-05-xx - xx - lavf 58.15.100 - avformat.h
> >> +  Add pmt_version field to AVProgram
> >> +  Add program_num, pmt_version, pmt_stream_idx to AVStream
> >> +
> >>  2018-05-xx - xx - lavf 58.14.100 - avformat.h
> >>Add AV_DISPOSITION_STILL_IMAGE
> >>
> >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >> index 6dce88fad5..ade918f99c 100644
> >> --- a/libavformat/avformat.h
> >> +++ b/libavformat/avformat.h
> >> @@ -1103,6 +1103,13 @@ typedef struct AVStream {
> >>   */
> >>  int stream_identifier;
> >>
> >> +/**
> >> + * Details of the MPEG-TS program which created this stream.
> >> + */
> >> +int program_num;
> >> +int pmt_version;
> >> +int pmt_stream_idx;
> >> +
> >>  int64_t interleaver_chunk_size;
> >>  int64_t interleaver_chunk_duration;
> >>
> >
> > These are added below the "All fields below this line are not part of
> the public API."
> > This contradicts the addition to APIChanges
>
> If these don't need to be accessed by the API user then I'd rather keep
> them here than moving them to the public section. Just remove the
> APIChanges entry.
> Same thing with the AVStream pmt_version field. If direct access is not
> needed for it, then it should be moved below the public notice.
>
> These fields seem pretty specific to one demuxer so ideally they
> shouldn't be public in case changes to them and the implementation are
> ever needed.


Yes they are specific to mpegts, and also may need to change in the future.

These were intended to be used in conjunction with the existing
AVStream.stream_identifier, which is also mpegts-specific. It appears in
the private section, even though the field is only written to from mpegts.c
and never used anywhere else in avformat. Clearly it was added to be used
by API users, but it too lives in the private section.

It seems like the best course forward is to remove APIchanges entries. The
fields are there if a user really needs to access them, but they are
considered private which means we can change them later if need be to
evolve support. In this case, should I revert the version bump as well? Or
is that still required since the ABI changed (even though it was in the
private section).

Aman


> ___
> 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 6/8] ffserver.c: Add config file reading

2018-05-20 Thread Stephan Holljes 
Signed-off-by: Stephan Holljes 
---
 ffserver.c | 248 ++---
 1 file changed, 172 insertions(+), 76 deletions(-)

diff --git a/ffserver.c b/ffserver.c
index 44fc263..3d842d8 100644
--- a/ffserver.c
+++ b/ffserver.c
@@ -38,6 +38,7 @@
 #include "segment.h"
 #include "publisher.h"
 #include "httpd.h"
+#include "configreader.h"
 
 #define BUFFER_SECS 30
 #define LISTEN_TIMEOUT_MSEC 1000
@@ -54,9 +55,11 @@ struct WriteInfo {
 };
 
 struct AcceptInfo {
-struct PublisherContext *pub;
+struct PublisherContext **pubs;
 struct HTTPDInterface *httpd;
-AVFormatContext *ifmt_ctx;
+AVFormatContext **ifmt_ctxs;
+struct HTTPDConfig *config;
+int nb_pub; /* number of publishers (streams) equal to number of ifmt_ctx 
*/
 };
 
 
@@ -287,52 +290,77 @@ void *accept_thread(void *arg)
 {
 struct AcceptInfo *info = (struct AcceptInfo*) arg;
 struct FFServerInfo *ffinfo = NULL;
+struct PublisherContext *pub;
 char status[4096];
+char *stream_name;
 struct HTTPClient *client = NULL;
 void *server = NULL;
 AVIOContext *client_ctx = NULL;
 AVFormatContext *ofmt_ctx = NULL;
+AVFormatContext *ifmt_ctx;
 unsigned char *avio_buffer;
 AVOutputFormat *ofmt;
 AVDictionary *mkvopts = NULL;
 AVStream *in_stream, *out_stream;
 int ret, i, reply_code;
-struct HTTPDConfig config = {
-.bind_address = "0",
-.port = 8080,
-.accept_timeout = LISTEN_TIMEOUT_MSEC,
-};
-
-info->httpd->init(, config);
-
-
+int shutdown;
+struct HTTPDConfig *config = info->config;
+
+info->httpd->init(, *config);
+
 for (;;) {
-if (info->pub->shutdown)
+shutdown = 1;
+for (i = 0; i < config->nb_streams; i++) {
+if (info->pubs[i] && !info->pubs[i]->shutdown)
+shutdown = 0;
+}
+if (shutdown)
 break;
-publisher_gen_status_json(info->pub, status);
-av_log(server, AV_LOG_INFO, status);
+for (i = 0; i < config->nb_streams; i++) {
+publisher_gen_status_json(info->pubs[i], status);
+av_log(server, AV_LOG_INFO, status);
+}
 client = NULL;
 av_log(server, AV_LOG_DEBUG, "Accepting new clients.\n");
 reply_code = 200;
-if (publisher_reserve_client(info->pub)) {
-av_log(client, AV_LOG_WARNING, "No more client slots free, 
Returning 503.\n");
-reply_code = 503;
-}
-
+
 if ((ret = info->httpd->accept(server, , reply_code)) < 0) {
 if (ret == HTTPD_LISTEN_TIMEOUT) {
-publisher_cancel_reserve(info->pub);
 continue;
 } else if (ret == HTTPD_CLIENT_ERROR) {
 info->httpd->close(server, client);
 }
 av_log(server, AV_LOG_WARNING, "Error during accept, retrying.\n");
-publisher_cancel_reserve(info->pub);
 continue;
 }
-
+
+pub = NULL;
+ifmt_ctx = NULL;
+for (i = 0; i < config->nb_streams; i++) {
+stream_name = info->pubs[i]->stream_name;
+//   skip leading '/'  ---v
+if(!strncmp(client->resource + 1, stream_name, 
strlen(stream_name))) {
+pub = info->pubs[i];
+ifmt_ctx = info->ifmt_ctxs[i];
+break;
+}
+}
+
+if (!pub || !ifmt_ctx) {
+av_log(client_ctx, AV_LOG_WARNING, "No suitable publisher found 
for resource: %s.\n",
+client->resource ? 
client->resource : "(null)");
+reply_code = 404;
+}
+
+
+if (pub && ifmt_ctx && publisher_reserve_client(pub)) {
+av_log(client_ctx, AV_LOG_WARNING, "No more client slots free, 
Returning 503.\n");
+reply_code = 503;
+}
+
 if (reply_code != 200) {
-publisher_cancel_reserve(info->pub);
+if (pub && ifmt_ctx)
+publisher_cancel_reserve(pub);
 info->httpd->close(server, client);
 continue;
 }
@@ -345,7 +373,7 @@ void *accept_thread(void *arg)
 client_ctx = avio_alloc_context(avio_buffer, AV_BUFSIZE, 1, ffinfo, 
NULL, _write, NULL);
 if (!client_ctx) {
 av_log(client, AV_LOG_ERROR, "Could not allocate output format 
context.\n");
-publisher_cancel_reserve(info->pub);
+publisher_cancel_reserve(pub);
 info->httpd->close(server, client);
 av_free(client_ctx->buffer);
 avio_context_free(_ctx);
@@ -355,7 +383,7 @@ void *accept_thread(void *arg)
 avformat_alloc_output_context2(_ctx, NULL, "matroska", NULL);
 if (!ofmt_ctx) {
 av_log(client, AV_LOG_ERROR, "Could not allocate output format 
context.\n");
-

[FFmpeg-devel] [PATCH 1/8] ffserver.c: Different timestamp scaling, still does not properly work with mp4 files?

2018-05-20 Thread Stephan Holljes 
Signed-off-by: Stephan Holljes 
---
 ffserver.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/ffserver.c b/ffserver.c
index 39e1c32..bd7c694 100644
--- a/ffserver.c
+++ b/ffserver.c
@@ -127,13 +127,13 @@ void *read_thread(void *arg)
 pkt.dts = 0;
 }
 
-pkt.pts = av_rescale_q_rnd(pkt.pts, in_stream->time_base, tb, 
AV_ROUND_NEAR_INF|AV_ROUND_PASS_MINMAX);
-pkt.dts = av_rescale_q_rnd(pkt.dts, in_stream->time_base, tb, 
AV_ROUND_NEAR_INF|AV_ROUND_PASS_MINMAX);
-pkt.duration = av_rescale_q(pkt.duration, in_stream->time_base, tb);
-pkt.pos = -1;
+//pkt.pts = av_rescale_q_rnd(pkt.pts, in_stream->time_base, tb, 
AV_ROUND_NEAR_INF|AV_ROUND_PASS_MINMAX);
+//pkt.dts = av_rescale_q_rnd(pkt.dts, in_stream->time_base, tb, 
AV_ROUND_NEAR_INF|AV_ROUND_PASS_MINMAX);
+//pkt.duration = av_rescale_q(pkt.duration, in_stream->time_base, tb);
+//pkt.pos = -1;
 
 // current pts
-pts = pkt.pts; //av_rescale_q(pkt.pts, in_stream->time_base, tb);
+pts = av_rescale_q(pkt.pts, in_stream->time_base, tb);
 
 // current stream "uptime"
 now = av_gettime_relative() - start;
@@ -249,8 +249,13 @@ void write_segment(struct Client *c)
 if (ret < 0)
 break;
 
-pkt.dts = seg->ts[pkt_count];
-pkt.pts = seg->ts[pkt_count+1];
+pkt.dts = av_rescale_q_rnd(seg->ts[pkt_count], 
fmt_ctx->streams[pkt.stream_index]->time_base,
+
c->ofmt_ctx->streams[pkt.stream_index]->time_base,
+   
AV_ROUND_NEAR_INF|AV_ROUND_PASS_MINMAX);
+pkt.pts = av_rescale_q_rnd(seg->ts[pkt_count+1], 
fmt_ctx->streams[pkt.stream_index]->time_base,
+
c->ofmt_ctx->streams[pkt.stream_index]->time_base,
+   
AV_ROUND_NEAR_INF|AV_ROUND_PASS_MINMAX);
+pkt.pos = -1;
 pkt_count += 2;
 ret = av_write_frame(c->ofmt_ctx, );
 av_packet_unref();
-- 
2.16.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 7/8] Makefile: Update Makefile

2018-05-20 Thread Stephan Holljes 
Signed-off-by: Stephan Holljes 
---
 Makefile | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index fbecdeb..83bc4e0 100644
--- a/Makefile
+++ b/Makefile
@@ -1,10 +1,11 @@
 all: ffserver
 LAV_FLAGS = $(shell pkg-config --libs --cflags libavformat libavcodec 
libavutil)
+LUA_FLAGS = $(shell pkg-config --libs --cflags lua5.3)
 CFLAGS=-fsanitize=address -fsanitize=undefined
 # LAV_FLAGS = -L/usr/local/lib -lavcodec -lavformat -lavutil
 
-ffserver: segment.o publisher.o lavfhttpd.o ffserver.c
-   cc -g -Wall $(CFLAGS) $(LAV_FLAGS) -lpthread -o ffserver segment.o 
publisher.o lavfhttpd.o ffserver.c
+ffserver: segment.o publisher.o lavfhttpd.o configreader.o ffserver.c
+   cc -g -Wall $(CFLAGS) $(LAV_FLAGS) $(LUA_FLAGS) -lpthread -o ffserver 
segment.o publisher.o lavfhttpd.o configreader.o ffserver.c
 
 segment.o: segment.c segment.h
cc -g -Wall $(CFLAGS) $(LAV_FLAGS) -lpthread -c segment.c
@@ -15,5 +16,7 @@ publisher.o: publisher.c publisher.h
 lavfhttpd.o: lavfhttpd.c httpd.h
cc -g -Wall $(CFLAGS) $(LAV_FLAGS) -lpthread -c lavfhttpd.c
 
+configreader.o: configreader.c configreader.h httpd.h
+   cc -g -Wall $(CFLAGS) $(LAV_FLAGS) $(LUA_FLAGS) -c configreader.c
 clean:
rm -f *.o ffserver
-- 
2.16.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [GSoC] FFserver config file support

2018-05-20 Thread Stephan Holljes 
This patch set builds upon the previous set that implemented ffserver
with a more flexible httpd interface.
It includes a fix for timestamp handling, which sadly still does not
properly fix the issue.

The majority of this patch set adds the ability to read lua-config
files. After reading up on how lua can be used for configuration files
and exchanges with my mentor and other developers (on IRC) I came to the
conclusion that lua is suited for this. The supplied sample_config.lua
should demonstrate that the configuration file is still non-confusing if
used in simple contexts, but can benefit from lua when defining more
complex setups.

As always, comments welcome, especially regarding the timestamp handling
as it still does not properly work with mp4 files :)

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 5/8] publisher.h: Add stream_name to PublisherContext

2018-05-20 Thread Stephan Holljes 
Signed-off-by: Stephan Holljes 
---
 publisher.c | 3 ++-
 publisher.h | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/publisher.c b/publisher.c
index 1123056..2e96f2f 100644
--- a/publisher.c
+++ b/publisher.c
@@ -93,11 +93,12 @@ void client_push_segment(struct Client *c, struct Segment 
*seg)
 client_set_state(c, WRITABLE);
 }
 
-void publisher_init(struct PublisherContext **pub)
+void publisher_init(struct PublisherContext **pub, char *stream_name)
 {
 int i;
 struct PublisherContext *pc = (struct PublisherContext*) 
av_malloc(sizeof(struct PublisherContext));
 pc->nb_threads = 8;
+pc->stream_name = stream_name;
 pc->current_segment_id = -1;
 pc->shutdown = 0;
 pc->buffer = av_fifo_alloc_array(sizeof(struct Segment), MAX_SEGMENTS);
diff --git a/publisher.h b/publisher.h
index 97b745d..e25c33d 100644
--- a/publisher.h
+++ b/publisher.h
@@ -73,6 +73,7 @@ struct PublisherContext {
 int nb_threads;
 int current_segment_id;
 int shutdown; // indicate shutdown, gracefully close client connections 
and files and exit
+char *stream_name;
 };
 
 /**
@@ -101,8 +102,9 @@ void client_set_state(struct Client *c, enum State state);
  * Allocate and initialize a PublisherContext
  *
  * @param pub pointer to a pointer to a PublisherContext. It will be allocated 
and initialized.
+ * @param stream_name string containing the name of the stream.
  */
-void publisher_init(struct PublisherContext **pub);
+void publisher_init(struct PublisherContext **pub, char *stream_name);
 
 /**
  * Push a Segment to a PublisherContext.
-- 
2.16.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v4 1/3] avformat: add fields to AVProgram/AVStream for PMT change tracking

2018-05-20 Thread Michael Niedermayer 
On Sun, May 20, 2018 at 11:37:22AM -0700, Aman Gupta wrote:
> On Sat, May 19, 2018 at 2:56 PM James Almer  wrote:
> 
> > On 5/19/2018 6:31 PM, Michael Niedermayer wrote:
> > > On Fri, May 18, 2018 at 11:15:04AM -0700, Aman Gupta wrote:
> > >> From: Aman Gupta 
> > >>
> > >> These fields will allow the mpegts demuxer to expose details about
> > >> the PMT/program which created the AVProgram and its AVStreams.
> > >>
> > >> In mpegts, a PMT which advertises streams has a version number
> > >> which can be incremented at any time. When the version changes,
> > >> the pids which correspond to each of it's streams can also change.
> > >>
> > >> Since ffmpeg creates a new AVStream per pid by default, an API user
> > >> needs the ability to (a) detect when the PMT changed, and (b) tell
> > >> which AVStream were added to replace earlier streams.
> > >>
> > >> This has been a long-standing issue with ffmpeg's handling of mpegts
> > >> streams with PMT changes, and I found two related patches in the wild
> > >> that attempt to solve the same problem.
> > >>
> > >> The first is in MythTV's ffmpeg fork, where they added a
> > >> void (*streams_changed)(void*); to AVFormatContext and call it from
> > >> their fork of the mpegts demuxer whenever the PMT changes.
> > >>
> > >> The second was proposed by XBMC in
> > >> https://ffmpeg.org/pipermail/ffmpeg-devel/2012-December/135036.html,
> > >> where they created a new AVMEDIA_TYPE_DATA stream with id=0 and
> > >> attempted to send packets to it whenever the PMT changed.
> > >>
> > >> Signed-off-by: Aman Gupta 
> > >> ---
> > >>  doc/APIchanges | 4 
> > >>  libavformat/avformat.h | 8 
> > >>  libavformat/utils.c| 1 +
> > >>  libavformat/version.h  | 2 +-
> > >>  4 files changed, 14 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/doc/APIchanges b/doc/APIchanges
> > >> index befa58c84a..a592073ca5 100644
> > >> --- a/doc/APIchanges
> > >> +++ b/doc/APIchanges
> > >> @@ -15,6 +15,10 @@ libavutil: 2017-10-21
> > >>
> > >>  API changes, most recent first:
> > >>
> > >> +2018-05-xx - xx - lavf 58.15.100 - avformat.h
> > >> +  Add pmt_version field to AVProgram
> > >> +  Add program_num, pmt_version, pmt_stream_idx to AVStream
> > >> +
> > >>  2018-05-xx - xx - lavf 58.14.100 - avformat.h
> > >>Add AV_DISPOSITION_STILL_IMAGE
> > >>
> > >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > >> index 6dce88fad5..ade918f99c 100644
> > >> --- a/libavformat/avformat.h
> > >> +++ b/libavformat/avformat.h
> > >> @@ -1103,6 +1103,13 @@ typedef struct AVStream {
> > >>   */
> > >>  int stream_identifier;
> > >>
> > >> +/**
> > >> + * Details of the MPEG-TS program which created this stream.
> > >> + */
> > >> +int program_num;
> > >> +int pmt_version;
> > >> +int pmt_stream_idx;
> > >> +
> > >>  int64_t interleaver_chunk_size;
> > >>  int64_t interleaver_chunk_duration;
> > >>
> > >
> > > These are added below the "All fields below this line are not part of
> > the public API."
> > > This contradicts the addition to APIChanges
> >
> > If these don't need to be accessed by the API user then I'd rather keep
> > them here than moving them to the public section. Just remove the
> > APIChanges entry.
> > Same thing with the AVStream pmt_version field. If direct access is not
> > needed for it, then it should be moved below the public notice.
> >
> > These fields seem pretty specific to one demuxer so ideally they
> > shouldn't be public in case changes to them and the implementation are
> > ever needed.
> 
> 
> Yes they are specific to mpegts, and also may need to change in the future.
> 
> These were intended to be used in conjunction with the existing
> AVStream.stream_identifier, which is also mpegts-specific. It appears in
> the private section, even though the field is only written to from mpegts.c
> and never used anywhere else in avformat. Clearly it was added to be used
> by API users, but it too lives in the private section.
> 

> It seems like the best course forward is to remove APIchanges entries. The
> fields are there if a user really needs to access them, but they are
> considered private which means we can change them later if need be to
> evolve support. In this case, should I revert the version bump as well? Or
> is that still required since the ABI changed (even though it was in the
> private section).

Iam not sure i understand what you suggest but if a field is added in the
private section then a user app cannot reliably access it. (It would move
in memory the next time a public field is added)

Also if the ABI changes in the future then any users using the API could break
(depending on the exact change)

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu


signature.asc
Description: PGP signature

[FFmpeg-devel] [PATCH 2/8] ffserver: Implement lua config file reader

2018-05-20 Thread Stephan Holljes 
Signed-off-by: Stephan Holljes 
---
 configreader.c | 211 +
 configreader.h |  46 +
 2 files changed, 257 insertions(+)
 create mode 100644 configreader.c
 create mode 100644 configreader.h

diff --git a/configreader.c b/configreader.c
new file mode 100644
index 000..84b27fa
--- /dev/null
+++ b/configreader.c
@@ -0,0 +1,211 @@
+/*
+ * 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 "configreader.h"
+#include "httpd.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+const char *stream_format_names[] = { "mkv" };
+
+void stream_free(struct StreamConfig *stream)
+{
+if (stream->stream_name)
+av_free(stream->stream_name);
+if (stream->input_uri)
+av_free(stream->input_uri);
+if (stream->formats)
+av_free(stream->formats);
+}
+
+void config_free(struct HTTPDConfig *config)
+{
+int i;
+if (config->server_name)
+av_free(config->server_name);
+if (config->bind_address)
+av_free(config->bind_address);
+if (config->streams) {
+for (i = 0; i < config->nb_streams; i++)
+stream_free(>streams[i]);
+av_free(config->streams);
+}
+}
+
+void config_dump(struct HTTPDConfig *config) {
+int i, j;
+printf("==\nserver name: %s\nbind_address: %s\nport: %d\nnb_streams: 
%d\n",
+config->server_name, config->bind_address, config->port, 
config->nb_streams);
+for (i = 0; i < config->nb_streams; i++) {
+printf("--\nstream_name: %s\ninput: %s\nformats: ",
+config->streams[i].stream_name, config->streams[i].input_uri);
+for (j = 0; j < config->streams[i].nb_formats; j++) {
+printf("%s ", stream_format_names[config->streams[i].formats[j]]);
+}
+printf("\n");
+}
+}
+
+int configs_read(struct HTTPDConfig **configs, const char *filename)
+{
+int ret = 0;
+int nb_configs = 0;
+int nb_streams = 0;
+int nb_formats = 0;
+int i;
+int index = 0;
+const char *key, *error;
+struct HTTPDConfig *parsed_configs = NULL;
+struct HTTPDConfig *config;
+struct StreamConfig *stream;
+lua_State *L = luaL_newstate();
+ret = luaL_loadfile(L, filename);
+if (ret != 0) {
+fprintf(stderr, "Unable to open config file: %s\n", lua_tostring(L, 
-1));
+lua_close(L);
+return -1;
+}
+
+ret = lua_pcall(L, 0, 0, 0);
+
+if (ret != 0) {
+fprintf(stderr, "Unable to read config file: %s\n", lua_tostring(L, 
-1));
+lua_close(L);
+return -1;
+}
+lua_getglobal(L, "settings");
+if (lua_type(L, -1) != LUA_TTABLE) {
+lua_pushstring(L, "Error \"settings\" is not a table");
+goto fail;
+}
+lua_pushnil(L);
+
+// iterate servers
+while (lua_next(L, -2) != 0) {
+nb_configs++;
+parsed_configs = av_realloc(parsed_configs, nb_configs * sizeof(struct 
HTTPDConfig));
+config = _configs[nb_configs - 1];
+config->server_name = NULL;
+config->bind_address = NULL;
+config->port = 0;
+config->accept_timeout = 1000;
+config->streams = NULL;
+config->nb_streams = 0;
+if (lua_type(L, -2) != LUA_TSTRING) {
+lua_pushstring(L, "Error server name is not a string.");
+goto fail;
+}
+if (lua_type(L, -1) != LUA_TTABLE) {
+lua_pushstring(L, "Error server settings is not a table.");
+goto fail;
+}
+config->server_name = av_strdup(lua_tostring(L, -2));
+lua_pushnil(L);
+// iterate server properties
+nb_streams = 0;
+while(lua_next(L, -2) != 0) {
+if (lua_type(L, -2) != LUA_TSTRING) {
+lua_pushstring(L, "Error server property is not a string.");
+goto fail;
+}
+key = lua_tostring(L, -2);
+if (!strncmp("bind_address", key, 12)) {
+config->bind_address = av_strdup(lua_tostring(L, -1));
+} else if (!strncmp("port", key, 4)) {
+config->port = (int) lua_tonumber(L, -1);
+} else {
+//

[FFmpeg-devel] [PATCH 8/8] doc: Update Documentation.txt and add sample config as supplement

2018-05-20 Thread Stephan Holljes 
Signed-off-by: Stephan Holljes 
---
 Documentation.txt | 17 -
 sample_config.lua | 28 
 2 files changed, 36 insertions(+), 9 deletions(-)
 create mode 100644 sample_config.lua

diff --git a/Documentation.txt b/Documentation.txt
index 9a7f0bf..c8fef11 100644
--- a/Documentation.txt
+++ b/Documentation.txt
@@ -67,16 +67,15 @@ struct HTTPDInterface {
 Usage
 -
 
-Currently streams can be supplied as a stream through stdin or any ffmpeg-
-compatible URI, e.g. files or network locations. Examples:
+To use ffserver, a config file has to be created that defines what streams to
+read and where to serve them. A sample config is supplied as sample_config.lua.
+This sample config defines two servers with a total of three streams. The first
+server serves two streams on all interfaces on port 8080, while the second
+server serves one stream on 127.0.0.1. The streams can be received by
+requesting the configured stream name as the GET parameter in the HTTP request.
+In the sample config, this would be 
"http://:8080/default_stream"
+for the first stream.
 
-cat somefile.mkv | ./ffserver
-
-./ffserver somefile.mkv
-
-./ffserver http://somehost/somefile.mkv
-
-This will start reading the file and open port 8080 for HTTP client 
connections.
 The stream is read in real time from whatever resource it is retrieved.
 Currently a maximum of 16 clients is implemented.
 
diff --git a/sample_config.lua b/sample_config.lua
new file mode 100644
index 000..a5e6192
--- /dev/null
+++ b/sample_config.lua
@@ -0,0 +1,28 @@
+-- Sample configuration file for ffserver
+-- Contains all settings
+settings = {
+-- A server instance
+default_server = {
+-- Server settings
+bind_address = "0.0.0.0",
+port = 8080,
+-- Stream settings
+default_stream = {
+input = "default.mkv",
+formats = { "mkv" }, -- later possible: { "mkv", "hls", "dash" }
+},
+other_stream = {
+input = "other_file.mkv",
+formats = { "mkv" }
+}
+},
+-- Another server instance
+other_server = {
+bind_address = "127.0.0.1",
+port = 8081,
+default_restream = {
+input = "yet_another_file.mkv",
+formats = { "mkv" }
+}
+}
+}
-- 
2.16.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 4/8] httpd.h: Adapt structs to config file

2018-05-20 Thread Stephan Holljes 
Signed-off-by: Stephan Holljes 
---
 httpd.h | 17 +
 1 file changed, 17 insertions(+)

diff --git a/httpd.h b/httpd.h
index 6fb91bd..25cbe11 100644
--- a/httpd.h
+++ b/httpd.h
@@ -26,11 +26,28 @@
 
 #include "publisher.h"
 
+/* Supported stream formats, for now only matroska */
+enum StreamFormat {
+FMT_MATROSKA = 0,
+FMT_NB,
+};
+
+/* Stream Config struct */
+struct StreamConfig {
+char *stream_name;
+char *input_uri;
+enum StreamFormat *formats;
+int nb_formats;
+};
+
 /* HTTPD Config struct */
 struct HTTPDConfig {
+char *server_name;
 char *bind_address;
 int port;
 int accept_timeout;
+struct StreamConfig *streams;
+int nb_streams;
 };
 
 /* HTTPClient struct, this information is shared between ffserver and the 
httpd implementation */
-- 
2.16.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/8] ffserver: Implement lua config file reader

2018-05-20 Thread Michael Niedermayer 
On Sun, May 20, 2018 at 08:53:58PM +0200, Stephan Holljes wrote:
> Signed-off-by: Stephan Holljes 
> ---
>  configreader.c | 211 
> +
>  configreader.h |  46 +
>  2 files changed, 257 insertions(+)
>  create mode 100644 configreader.c
>  create mode 100644 configreader.h
> 
> diff --git a/configreader.c b/configreader.c
> new file mode 100644
> index 000..84b27fa
> --- /dev/null
> +++ b/configreader.c
> @@ -0,0 +1,211 @@
[...]
> +int configs_read(struct HTTPDConfig **configs, const char *filename)
> +{
> +int ret = 0;
> +int nb_configs = 0;
> +int nb_streams = 0;
> +int nb_formats = 0;
> +int i;
> +int index = 0;
> +const char *key, *error;
> +struct HTTPDConfig *parsed_configs = NULL;
> +struct HTTPDConfig *config;
> +struct StreamConfig *stream;
> +lua_State *L = luaL_newstate();
> +ret = luaL_loadfile(L, filename);
> +if (ret != 0) {
> +fprintf(stderr, "Unable to open config file: %s\n", lua_tostring(L, 
> -1));
> +lua_close(L);
> +return -1;
> +}
> +
> +ret = lua_pcall(L, 0, 0, 0);
> +
> +if (ret != 0) {
> +fprintf(stderr, "Unable to read config file: %s\n", lua_tostring(L, 
> -1));
> +lua_close(L);
> +return -1;
> +}
> +lua_getglobal(L, "settings");
> +if (lua_type(L, -1) != LUA_TTABLE) {
> +lua_pushstring(L, "Error \"settings\" is not a table");
> +goto fail;
> +}
> +lua_pushnil(L);
> +
> +// iterate servers
> +while (lua_next(L, -2) != 0) {
> +nb_configs++;

> +parsed_configs = av_realloc(parsed_configs, nb_configs * 
> sizeof(struct HTTPDConfig));
> +config = _configs[nb_configs - 1];

Missing realloc failure handling


> +config->server_name = NULL;
> +config->bind_address = NULL;
> +config->port = 0;
> +config->accept_timeout = 1000;
> +config->streams = NULL;
> +config->nb_streams = 0;
> +if (lua_type(L, -2) != LUA_TSTRING) {
> +lua_pushstring(L, "Error server name is not a string.");
> +goto fail;
> +}
> +if (lua_type(L, -1) != LUA_TTABLE) {
> +lua_pushstring(L, "Error server settings is not a table.");
> +goto fail;
> +}

> +config->server_name = av_strdup(lua_tostring(L, -2));

Missing alloc failure check, same with other similar allocation

[...]
> +fail:
> +error = lua_tostring(L, -1);
> +fprintf(stderr, "%s\n", error);
> +lua_close(L);
> +for (i = 0; i < nb_configs; i++)
> +config_free(_configs[i]);
> +av_free(parsed_configs);

> +return -1;
> +}

We probably should use error codes (AVERROR*) from the begin.
Otherwise it would need a "2nd" pass one day to cleanup


> +
> diff --git a/configreader.h b/configreader.h
> new file mode 100644
> index 000..788ff60
> --- /dev/null
> +++ b/configreader.h
> @@ -0,0 +1,46 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +#ifndef CONFIGREADER_H
> +#define CONFIGREADER_H
> +
> +#include "httpd.h"
> +
> +/**
> + * Read configurations from a file using the json format. The configurations
> + * are allocated as an array at *configs. This has to be freed by the user.
> + *
> + * @param configs pointer to a pointer where configurations will be 
> allocated.
> + * @param filename filename of the configuration to use.
> + * @return number of configurations read, -1 on error.
> + */
> +int configs_read(struct HTTPDConfig **configs, const char *filename);
> +
> +/**
> + * Dump a configuration to stdout.
> + * @param config pointer to a configuration
> + */
> +void config_dump(struct HTTPDConfig *config);

it may be more flexible to add a argument that can point to stdout/stderr/file

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.

Re: [FFmpeg-devel] [PATCH v2 1/5] avcodec/vc1: FIELDTX is only present in interlaced frame I/BI pictures

2018-05-20 Thread Michael Niedermayer 
On Sun, May 20, 2018 at 01:45:56PM +0200, Jerome Borsboom wrote:
> If v->fieldtx_is_raw is not reset to zero, it may spill over from a previous
> interlaced frame I/BI picture.
> 
> Signed-off-by: Jerome Borsboom 
> ---
> This may address the concerns. Will make a mental note to clean up the parser
> at a later time.
> 
> Thank you for the review.

will apply

thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/mov: replace a value error by clipping into valid range in mov_read_stsc()

2018-05-20 Thread Michael Niedermayer 
Fixes: #7165

Signed-off-by: Michael Niedermayer 
---
 libavformat/mov.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index a078bf4712..f2a540ad50 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2642,14 +2642,22 @@ static int mov_read_stsc(MOVContext *c, AVIOContext 
*pb, MOVAtom atom)
 
 sc->stsc_count = i;
 for (i = sc->stsc_count - 1; i < UINT_MAX; i--) {
+int64_t first_min = i + 1;
 if ((i+1 < sc->stsc_count && sc->stsc_data[i].first >= 
sc->stsc_data[i+1].first) ||
 (i > 0 && sc->stsc_data[i].first <= sc->stsc_data[i-1].first) ||
-sc->stsc_data[i].first < 1 ||
+sc->stsc_data[i].first < first_min ||
 sc->stsc_data[i].count < 1 ||
 sc->stsc_data[i].id < 1) {
 av_log(c->fc, AV_LOG_WARNING, "STSC entry %d is invalid (first=%d 
count=%d id=%d)\n", i, sc->stsc_data[i].first, sc->stsc_data[i].count, 
sc->stsc_data[i].id);
-if (i+1 >= sc->stsc_count || sc->stsc_data[i+1].first < 2)
-return AVERROR_INVALIDDATA;
+if (i+1 >= sc->stsc_count) {
+sc->stsc_data[i].first = FFMAX(sc->stsc_data[i].first, 
first_min);
+if (i > 0 && sc->stsc_data[i].first <= 
sc->stsc_data[i-1].first)
+sc->stsc_data[i].first = FFMIN(sc->stsc_data[i-1].first + 
1LL, INT_MAX);
+sc->stsc_data[i].count = FFMAX(sc->stsc_data[i].count, 1);
+sc->stsc_data[i].id= FFMAX(sc->stsc_data[i].id, 1);
+continue;
+}
+av_assert0(sc->stsc_data[i+1].first >= 2);
 // We replace this entry by the next valid
 sc->stsc_data[i].first = sc->stsc_data[i+1].first - 1;
 sc->stsc_data[i].count = sc->stsc_data[i+1].count;
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] avformat/mov: Only fail for STCO/STSC contradictions if both exist

2018-05-20 Thread Michael Niedermayer 
On Tue, May 15, 2018 at 06:53:49PM +0200, Carl Eugen Hoyos wrote:
> 2018-05-15 17:07 GMT+02:00, Michael Niedermayer :
> > Fixes regression with playback of
> > GF9720Repeal20the20Eighth20with20Helen20Linehan.m4a
> > See: crbug 822666
> 
> Please mention ticket #7165 if related.

its unrelated, ill post a patch fixing this if it passes tests

will apply this patchset

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel