Zms/videostream improvements

[Sune1337]

Support streaming with zms (using ffmpeg) better …

* ZoneMinder compiles with ffmpeg 0.5..master
* Make VideoStream class able to stream through ffmpeg better.
* Ability to use fixed quality instead of fixed bitrate (by specifying bitrate 0...100)
* Format url parameter supports <format>/<encoder> syntax

See commit message for details.

[Sune1337]

This is the cleaned up version of #382 as agreed!

[Sune1337]

A good format to use is "&format=mpegts/mpeg4". It works pretty long way back with FFmpeg. Personally i cannot use the ZoneMinder default "mpeg" since my FFmpeg can't encode 5FPS using that.

Otherwise the point of this request is to give the user (me :)) better control for his/hers own purposes.

[Sune1337]

The two last commits (4767326 and 3155d62) is "bugfixes" to the first commits. Consider it part of the first commit.

[Sune1337]

When you say it like that it sounds so obvious :)

[knight-of-ni]

@Sune1337 he does that to me too :-)

Not sure if you know this, but we are usually online on the #zoneminder freenode irc channel. You are welcome to join us.

[knight-of-ni]

Unless I hear otherwise from someonese else, I am going to give this a shot at merging this weekend.

It can't be auto-merged so I'll have to manually address whatever merge conflicts exists.

When it comes to troubleshooting zoneminder's streaming video C code, my understanding of what is going on is very limited. Consequently, @Sune1337 if you have a second and can take a look at what the problem might be, you are more likely to be able to fix the issue much faster than I can.

Anyway, I'll give it a shot.

[Sune1337]

I had some uncommitted code in this branch; im gonna investigate what that is tomow. I'll also check out the conflicts.

[Sune1337]

My uncommitted changes is an attempt to implement variable framerate; which failed at the time. those are not important for this pull-request, so i'll just keep that experiment locally.

I'll check out the conflicts now.

[Sune1337]

I have resolved the conflicts.

as in #351 i will await confirmation that checking in alot of files in this branch will actually help you merge, and not make thins messier :)

To repeat: If i commit now i'll commit changes to a lot of files. I guess these are the changes between this branch and the ZoneMinder-master. Is this correct? Should i do the commit and push?

[Sune1337]

I'll paste files to help you resolve conflicts on this pull request too.

What i did: git clone https://github.com/ZoneMinder/ZoneMinder -b master ZoneMinder-master git remote add upstream https://github.com/Sune1337/ZoneMinder -t zms/videostream-improvements git fetch upstream git merge upstream/zms/videostream-improvements git mergetool

Resolves conflicts: src/zm_ffmpeg.h: http://pastebin.com/N1r19hTH src/zm_mpeg.cpp: http://pastebin.com/kRjgkZDp

edit: the zm_ffmpeg.h pastebin had been deleted somehow. updated the link.

[knight-of-ni]

This cannot be auto-merged due to conflicts with the following pull request that has already been merged: #325 I suspected that was the pull request that caused this.

There appear to be just two conflicts, and we need to figure out how to merge your changes in w/o unravelling the changes from that pull request.

For the conflict in zm_mpeg.cpp, you are using _AVCODECID_NONE while the author of #325 chose to use AV_CODEC_ID_NONE. Another example uses "of->video_codec" while you are using "codec_id". I don't know exactly what the end results should be.... probably a merge of your changes and his, but that's all I know right now.

Can you compare what is going on in #325 and then tell me how this new code should look? You may want to consider making a new pull request.

[Sune1337]

I also saw #325. I have accounted for this in the pastebin files (I use the AV_CODEC_ID_NONE as the author of #325 does).

Regarding the use of video_codec vs codec_id i'm not sure what you mean. Do you mean that i use CODECID* macros instead of AV_CODECID* ?

[knight-of-ni]

No, it means I just don't know what I am looking at and didn't understand which to use.

[Sune1337]

I gave you 2 pastebin files above which i intended for you to use when resolving the conflicts. Isn't it the zm_ffmpeg.h and zm_mpeg.cpp that conflicts?

[knight-of-ni]

I'm just trying to understand it first. Give me a minute.

[knight-of-ni]

Arrrghh - this doesn't build.

/home/abauer/rpmbuild/BUILD/ZoneMinder-1.27.99/src/zm_ffmpeg.h:48:83: error: missing binary operator before token "("
 #if defined(HAVE_LIBAVCODEC_AVCODEC_H) && LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(54,25,0)
                                                                                   ^
make[2]: *** [src/CMakeFiles/zm.dir/zm_curl_camera.cpp.o] Error 1

libavutil 52. 48.101 / 52. 48.101 libavcodec 55. 39.101 / 55. 39.101 libavformat 55. 19.104 / 55. 19.104 libavdevice 55. 5.100 / 55. 5.100 libavfilter 3. 90.100 / 3. 90.100 libavresample 1. 1. 0 / 1. 1. 0 libswscale 2. 5.101 / 2. 5.101 libswresample 0. 17.104 / 0. 17.104 libpostproc 52. 3.100 / 52. 3.100

[knight-of-ni]

This does not build on Fedora 20 w/ ffmpeg 2.1.1 (error is in previous post) This builds on CentOS 6 w/ ffmpeg 0.10.15 Travis built this successfully on Ubuntu w/ ffmpeg 2.4.1

[knight-of-ni]

Further progress... forgot that my build environment for Fedora tries to build w/o ffmpeg by default. If I enable ffmpeg support, it builds fine. What is broken is the ability to build zoneminder w/o any ffmpeg support... that explains why it could not find AV_VERSION_INT.

[Sune1337]

Do you want me to create a pull request that can compile w/o ffmpeg?

[Sune1337]

shouldn't "#if defined(HAVE_LIBAVCODEC_AVCODEC_H) && ..." make it work without ffmpeg?

[knight-of-ni]

I guess the compiler is still trying to evaluate the portion to the right of &&, even though the portion to the left is false. My wife just came home so I need to step away for a bit.

[Sune1337]

ok; if you just wanna commit a fix for it when you have time, i fixed the issue with the following code.

#if defined(HAVE_LIBAVCODEC_AVCODEC_H)
#if LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(54,25,0)
    #define _AVCODECID AVCodecID
#else
    #define _AVCODECID CodecID
#endif
#endif

instead of

#if defined(HAVE_LIBAVCODEC_AVCODEC_H) && LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(54,25,0)
    #define _AVCODECID AVCodecID
#else
    #define _AVCODECID CodecID
#endif

[knight-of-ni]

fixed. Thanks.

[knight-of-ni]

@Sune1337 Perhaps you can help me with something that is semi-related to this pullrequest.

When ffmpeg introducess a new function call, how can one determine in which version of ffmpeg (or the relevant avxxx library) it was introduced?

Take avformat_alloc_output_context2 for example. How do I figure out what version of ffmpeg or avformat this was introduced, without manually browsing through many ffmpeg github commits?

[SteveGilvarry]

Hope you don't mind if I answer but I needed the same thing earlier in the week. https://github.com/FFmpeg/FFmpeg/blob/master/doc/APIchanges Search for avformat_alloc_output_context2

2011-05-22 - 5ecdfd0 - lavf 53.2.0 - avformat.h Introduce avformat_alloc_output_context2() and deprecate avformat_alloc_output_context().

[Sune1337]

thats a nice way i didnt know of. i use the blame function to find what commit something comes from.. tedious. ill try the APIchanges doc next time!

[knight-of-ni]

Yeah, that was really helpful. I knew there had to be a faster way. Each time I Googled I ended up finding the definition but not when it was introduced.

Anyway, today we discovered that avformat_alloc_output_context2 is not implemented in libav, Debian's version of ffmpeg that is supposed to be identical but isn't. wtf. This causes our deb package builds to fail.

What we need to do is make sure the following conditional is false when building against libav instead of ffmpeg: https://github.com/ZoneMinder/ZoneMinder/blob/master/src/zm_mpeg.cpp#L61

I just learned that ffmpeg is using a "micro" version number >= 100 while libav is >= 0. That might help. Here is an example of how to check for libav/ffmpeg with autotools, but I'm not yet convinced we need something so complicated: https://code.google.com/p/ffmpegsource/source/browse/trunk/configure.ac?r=736#144

UPDATE: I think I might just tack on "&& LIBAVFORMAT_VERSION_MICRO >= 100"

[SteveGilvarry]

Here is an example of how VLC did it, which was as you say using micro version, but this way would work for API changes in different minor versions.

#ifdef HAVE_LIBAVFORMAT_AVFORMAT_H
# include <libavformat/avformat.h>

#define LIBAVFORMAT_VERSION_CHECK( a, b, c, d, e ) \
    ( (LIBAVFORMAT_VERSION_MICRO <  100 && LIBAVFORMAT_VERSION_INT >= AV_VERSION_INT( a, b, c ) ) || \
      (LIBAVFORMAT_VERSION_MICRO >= 100 && LIBAVFORMAT_VERSION_INT >= AV_VERSION_INT( a, d, e ) ) )

#endif

https://github.com/videolan/vlc/blob/d36bc0a71a7a69afd085c8b2754ecfbc5876fd2b/modules/codec/avcodec/avcommon_compat.h

[SteveGilvarry]

I should have added this also as it was something that I was looking at for a bunch of the Deprecated messages we get.

#if LIBAVCODEC_VERSION_INT < AV_VERSION_INT(55,28,1)
#define av_frame_alloc  avcodec_alloc_frame
#endif

Where ever the API is a simple name change we should use this to limit the number of #if LIBAVCODEC_VERSION_INT < AV_VERSION_INT sections in the code.

I might come back to this one post 1.28 and have a go at cleaning up.

[Sune1337]

it seems the avformat_alloc_output_context2 isn't the only function not supported by libav (the fork). Next issue is av_opt_set_defaults, then avcodec_get_name. then i didn't look more..

[Sune1337]

I don't like libav now.

[Sune1337]

I created https://github.com/Sune1337/ZoneMinder_dup/tree/libav_the_fork_compat which compiles iwth libav. ZMS do however coredump if one is trying to stream using libav video.

[knight-of-ni]

I wonder if we should just revert back to the old code path if building against libav. We know that worked, right? We could always come back to this and improve it after we release 1.28.0.

[Sune1337]

the coredump was a bug; should exist when compiling with ffmpeg too.

[Sune1337]

ps. i never got the zms to stream video before this pullrequest. ever. and i never saw anyone claiming they did either,, so i don't agree with old code working.

[Sune1337]

i could test-stream now with libav using AVI format. seems there are some differences in mpegts handling (which was the other format i tested)

[Sune1337]

do you want me to create pull-request on the branch? If nothing else, to suggest a solution to compiling with libav.

[knight-of-ni]

Yeah, and I'll ask @connortechnology to test. I don't have an environment to test libav from so I'm going to be relying on others to tell what works and what doesn't.

Also, can you make line 61 do this?

#if ((LIBAVFORMAT_VERSION_INT >= AV_VERSION_INT(53, 5, 0)) && (LIBAVFORMAT_VERSION_MICRO >= 100))

I did like the extra info in those debug statements from your original pull request. Maybe we can get those back in sometime later after we develop a compatible solution.

[Sune1337]

Do you mean change zm_ffmpege.cpp:61 from:

    AVFormatContext *s= avformat_alloc_context();

into

#if ((LIBAVFORMAT_VERSION_INT >= AV_VERSION_INT(53, 5, 0)) && (LIBAVFORMAT_VERSION_MICRO >= 100))
    avformat_alloc_output_context2( &ofc, NULL, format, filename );
#else
    AVFormatContext *s= avformat_alloc_context();

[Sune1337]

btw; can you use 'LIBAVFORMAT_VERSION_MICRO >= 100' to determine if we're using libav or ffmpeg?

[knight-of-ni]

Yes.

I was looking at zm_ffmpeg.cpp from your branch: https://github.com/Sune1337/ZoneMinder_dup/blob/master/src/zm_mpeg.cpp#L61

We want that condition to be false if using libav, but I'm assuming we are looking for a solution that uses avformat_alloc_output_context2 but only when building against ffmpeg.

[knight-of-ni]

You can use LIBAVFORMAT_VERSION_MICRO >= 100 but it may not be that simple depending on what you are trying to do.

[Sune1337]

oh, the changes is in a branch called 'libav_the_fork_compat'.

But i'm gonna use the 'LIBAVFORMAT_VERSION_MICRO >= 100' method instead as you suggested. I agree this is better.

[knight-of-ni]

@SteveGilvarry posted a good example from libvlc earlier, but I think in our particular case we can just tack on LIBAVFORMAT_VERSION_MICRO.

In time if we run into more of these, then we may have to look into doing something more encompasing like in libvlc library.

[Sune1337]

So; i have started to get stuff sorted out :) I'll delete the branch i created above and create a new one to get stuff fixed the correct way.

[Sune1337]

Man, github is messing with me.

To be able to create a branch for the fixes, i needed to fork the ZoneMinder repo again. I needed to do this since i cant remove my existing one. I cant use my existing one because i have an active pull-request on that. Phew.

github.com does not allow one to create a second fork, so i had to do this manually.

Now i cant create pull-requests on the new fork; github.com isnt aware of the "connection".

Can you use https://github.com/Sune1337/ZoneMinder_dup/tree/libav_the_fork_compat and create some manual pull request?

https://github.com/Sune1337/ZoneMinder_dup/tree/libav_the_fork_compat now contains what i think is the correct way of adressing the issues caused by #404 in regards to libav the fork.

[Sune1337]

i just pushed a new commit "compile with ffmpeg <= 0.8". Found issues with older FFmpegs.

[knight-of-ni]

Yeah, that's why you don't create pull requests against your own master branch.

When viewing a commit, one can add ".patch" to the url and github will auto-create a patch. If you can identify exactly which commits need to be applied, then maybe we can get @connortechnology to patch his source and test.

[Sune1337]

It's all the commits in the branch "libav_the_fork_compat"; here are direct links to all patch files.

https://github.com/Sune1337/ZoneMinder_dup/commit/6e77632a5946097bcb5489dce999814f1110716b.patch https://github.com/Sune1337/ZoneMinder_dup/commit/4b47336778aa76050aa6110d0420de8413586195.patch https://github.com/Sune1337/ZoneMinder_dup/commit/df4a9f27751592655f2e1a20e58079c7d2985fe2.patch https://github.com/Sune1337/ZoneMinder_dup/commit/055506838d40834d104c60daf8e5f264828cb050.patch

[Sune1337]

The pull request on master branch is an old mistake i'll never forget. Even so, i guess there's nothing i can do about it as it's already done.