Re: [FFmpeg-devel] [PATCH 00/11] CRC32 support for Matroska muxer

[James Almer]

On 10/6/2016 6:10 PM, Dave Rice wrote:
> 
>> On Oct 6, 2016, at 4:33 PM, James Almer  wrote:
>>
>> On 10/6/2016 3:28 PM, Michael Niedermayer wrote:
>>> On Thu, Oct 06, 2016 at 11:27:08AM -0300, James Almer wrote:
 On 10/6/2016 7:29 AM, Michael Niedermayer wrote:
> On Mon, Oct 03, 2016 at 08:36:56PM -0300, James Almer wrote:
>> This patchsets implements the feature requested on ticket #4347.
>> The first three patches are preparation work. The first one isn't
>> strictly related to the implementation, but comes in handy
>> nonetheless.
>>
>> Patches 4 to 11 can be squashed into a single commit before pushing
>> if that's prefered, but for review purposes i split things as one
>> patch per Level 1 element being adapted to write a CRC32 element.
>>
>> Fate tests are updated as needed.
>
> some questions
> Does this reduce writing speed ? for example when remuxing high
> bitrate data like rawvideo ?

 The bulk of the CRC calculation is done once per level 1 element, so it
 shouldn't affect remuxing speed in a considerable way. But if preferred,
 i could see about adding an option to disable it.
>>>
>>> i guess such a option cant hurt
>>
>> Will add it.
> 
> Worth adding to the Changelog as well.

Added both the option and a Changelog entry.

> 
>>> thanks
>>>
>>> no furter comments from me about this patchset
>>
>> Pushed without squashing, so if problems arise bisect can more accurately
>> point where things went wrong.
>>
>> Thanks.
>>
>> ___
>> 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 mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 00/11] CRC32 support for Matroska muxer

[Dave Rice]

> On Oct 6, 2016, at 4:33 PM, James Almer  wrote:
> 
> On 10/6/2016 3:28 PM, Michael Niedermayer wrote:
>> On Thu, Oct 06, 2016 at 11:27:08AM -0300, James Almer wrote:
>>> On 10/6/2016 7:29 AM, Michael Niedermayer wrote:
 On Mon, Oct 03, 2016 at 08:36:56PM -0300, James Almer wrote:
> This patchsets implements the feature requested on ticket #4347.
> The first three patches are preparation work. The first one isn't
> strictly related to the implementation, but comes in handy
> nonetheless.
> 
> Patches 4 to 11 can be squashed into a single commit before pushing
> if that's prefered, but for review purposes i split things as one
> patch per Level 1 element being adapted to write a CRC32 element.
> 
> Fate tests are updated as needed.
 
 some questions
 Does this reduce writing speed ? for example when remuxing high
 bitrate data like rawvideo ?
>>> 
>>> The bulk of the CRC calculation is done once per level 1 element, so it
>>> shouldn't affect remuxing speed in a considerable way. But if preferred,
>>> i could see about adding an option to disable it.
>> 
>> i guess such a option cant hurt
> 
> Will add it.

Worth adding to the Changelog as well.

>> thanks
>> 
>> no furter comments from me about this patchset
> 
> Pushed without squashing, so if problems arise bisect can more accurately
> point where things went wrong.
> 
> Thanks.
> 
> ___
> 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] [PATCH 00/11] CRC32 support for Matroska muxer

[James Almer]

On 10/6/2016 3:28 PM, Michael Niedermayer wrote:
> On Thu, Oct 06, 2016 at 11:27:08AM -0300, James Almer wrote:
>> On 10/6/2016 7:29 AM, Michael Niedermayer wrote:
>>> On Mon, Oct 03, 2016 at 08:36:56PM -0300, James Almer wrote:
 This patchsets implements the feature requested on ticket #4347.
 The first three patches are preparation work. The first one isn't
 strictly related to the implementation, but comes in handy
 nonetheless.

 Patches 4 to 11 can be squashed into a single commit before pushing
 if that's prefered, but for review purposes i split things as one
 patch per Level 1 element being adapted to write a CRC32 element.

 Fate tests are updated as needed.
>>>
>>> some questions
>>> Does this reduce writing speed ? for example when remuxing high
>>> bitrate data like rawvideo ?
>>
>> The bulk of the CRC calculation is done once per level 1 element, so it
>> shouldn't affect remuxing speed in a considerable way. But if preferred,
>> i could see about adding an option to disable it.
> 
> i guess such a option cant hurt

Will add it.

> 
> thanks
> 
> no furter comments from me about this patchset

Pushed without squashing, so if problems arise bisect can more accurately
point where things went wrong.

Thanks.

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

Re: [FFmpeg-devel] [PATCH 00/11] CRC32 support for Matroska muxer

[Michael Niedermayer]

On Thu, Oct 06, 2016 at 11:27:08AM -0300, James Almer wrote:
> On 10/6/2016 7:29 AM, Michael Niedermayer wrote:
> > On Mon, Oct 03, 2016 at 08:36:56PM -0300, James Almer wrote:
> >> This patchsets implements the feature requested on ticket #4347.
> >> The first three patches are preparation work. The first one isn't
> >> strictly related to the implementation, but comes in handy
> >> nonetheless.
> >>
> >> Patches 4 to 11 can be squashed into a single commit before pushing
> >> if that's prefered, but for review purposes i split things as one
> >> patch per Level 1 element being adapted to write a CRC32 element.
> >>
> >> Fate tests are updated as needed.
> > 
> > some questions
> > Does this reduce writing speed ? for example when remuxing high
> > bitrate data like rawvideo ?
> 
> The bulk of the CRC calculation is done once per level 1 element, so it
> shouldn't affect remuxing speed in a considerable way. But if preferred,
> i could see about adding an option to disable it.

i guess such a option cant hurt

thanks

no furter comments from me about this patchset

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

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus


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

Re: [FFmpeg-devel] [PATCH 00/11] CRC32 support for Matroska muxer

[James Almer]

On 10/6/2016 7:29 AM, Michael Niedermayer wrote:
> On Mon, Oct 03, 2016 at 08:36:56PM -0300, James Almer wrote:
>> This patchsets implements the feature requested on ticket #4347.
>> The first three patches are preparation work. The first one isn't
>> strictly related to the implementation, but comes in handy
>> nonetheless.
>>
>> Patches 4 to 11 can be squashed into a single commit before pushing
>> if that's prefered, but for review purposes i split things as one
>> patch per Level 1 element being adapted to write a CRC32 element.
>>
>> Fate tests are updated as needed.
> 
> some questions
> Does this reduce writing speed ? for example when remuxing high
> bitrate data like rawvideo ?

The bulk of the CRC calculation is done once per level 1 element, so it
shouldn't affect remuxing speed in a considerable way. But if preferred,
i could see about adding an option to disable it.

> 
> does this increase latency as in case of streaming ?
> it seemed not from reading the code but iam asking anyway to double
> check

The code mentions the output protocol is not seekable in the case of
streaming. CRC is not calculated and the clusters are written the same
way as before in such scenarios, so it wouldn't make a difference at
all.

> 
> are there any cases where this unreasonably increases memory
> consumption ? (thinking of OOM issues here not a few %)
> probably not but again asking to be sure, you know the code as
> author better ...

Technically, only if the user gives an unreasonable cluster_size_limit
or cluster_time_limit value. But not even then because there are other
checks in the codebase to start new clusters at for example every
keyframe, or when a block's relative timestamp can't be stored as a
signed 2 byte value in the block's header (commit 7923aa0f).

> 
> [...]
> 
> 
> 
> ___
> 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] [PATCH 00/11] CRC32 support for Matroska muxer

[Jerome Martinez]

Le 06/10/2016 à 12:29, Michael Niedermayer a écrit :

Does this reduce writing speed ?


in the same manner as e.g. reducing FFV1 writing speed with default 
configuration, i.e. with CRC per slice (same kind of job).
On my machine (i7 from 2012), CRC computing takes less than 1% of the 
CPU when I decode FFV1, so I estimate the impact of CRC on both FFV1 
level and Matroska level to less than 2% in case FFV1 is used. I don't 
think it would be a more with e.g. AVC as frame byte size is smaller.



for example when remuxing high bitrate data like rawvideo ?


On my machine (i7 from 2012), with the "4 bytes at once" algorithm used 
by FFmpeg if I well understood FFmpeg code on this part, I benched CRC 
computing at 1 GB/s per core, so it should be more than enough also for 
rawvideo.


CRC element could be an option as it is for FFV1, but I argue for having 
it set by default, for the same reasons it is set by default with FFV1.



[...]


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

Re: [FFmpeg-devel] [PATCH 00/11] CRC32 support for Matroska muxer

[Michael Niedermayer]

On Mon, Oct 03, 2016 at 08:36:56PM -0300, James Almer wrote:
> This patchsets implements the feature requested on ticket #4347.
> The first three patches are preparation work. The first one isn't
> strictly related to the implementation, but comes in handy
> nonetheless.
> 
> Patches 4 to 11 can be squashed into a single commit before pushing
> if that's prefered, but for review purposes i split things as one
> patch per Level 1 element being adapted to write a CRC32 element.
> 
> Fate tests are updated as needed.

some questions
Does this reduce writing speed ? for example when remuxing high
bitrate data like rawvideo ?

does this increase latency as in case of streaming ?
it seemed not from reading the code but iam asking anyway to double
check

are there any cases where this unreasonably increases memory
consumption ? (thinking of OOM issues here not a few %)
probably not but again asking to be sure, you know the code as
author better ...

[...]
-- 
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: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 00/11] CRC32 support for Matroska muxer

[James Almer]

This patchsets implements the feature requested on ticket #4347.
The first three patches are preparation work. The first one isn't
strictly related to the implementation, but comes in handy
nonetheless.

Patches 4 to 11 can be squashed into a single commit before pushing
if that's prefered, but for review purposes i split things as one
patch per Level 1 element being adapted to write a CRC32 element.

Fate tests are updated as needed.

James Almer (11):
  avformat/matroskaenc: don't reserve space for stream duration tags if
the output is not seekable
  avformat/matroskaenc: print debug message with cluster offsets only if
the output is seekable
  avformat/matroskaenc: always use a dynamic buffer when writting
clusters
  avformat/matroskaenc: write a CRC32 element on each Cluster
  avformat/matroskaenc: write a CRC32 element on SeekHead
  avformat/matroskaenc: write a CRC32 element on Cues
  avformat/matroskaenc: write a CRC32 element on Tracks
  avformat/matroskaenc: write a CRC32 element on Chapters
  avformat/matroskaenc: write a CRC32 element on Attachments
  avformat/matroskaenc: write a CRC32 element on Tags
  avformat/matroskaenc: write a CRC32 element on Info

 libavformat/matroskaenc.c| 338 ---
 tests/fate/matroska.mak  |   2 +-
 tests/fate/wavpack.mak   |   4 +-
 tests/ref/fate/binsub-mksenc |   2 +-
 tests/ref/fate/rgb24-mkv |   4 +-
 tests/ref/lavf/mka   |   4 +-
 tests/ref/lavf/mkv   |   8 +-
 tests/ref/seek/lavf-mkv  |  44 +++---
 8 files changed, 225 insertions(+), 181 deletions(-)

-- 
2.9.1

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