linuxstb / pidvbip

tvheadend client for the Raspberry Pi
http://www.pidvbip.org
GNU General Public License v2.0
53 stars 22 forks source link

Build system cleanup #6

Closed saintdev closed 11 years ago

saintdev commented 11 years ago

Adds a configure script to automatically detect libraries and simplifies the Makefile.

I wasn't able to test building with libcec. The version that ships with arch attempts to define the same symbol in cectypes.h and cecc.h. It is correctly disabled in this case, at least.

I didn't touch the vgfont build system. My thinking was this would make syncing with upstream vgfont easier.

make now only builds pidvbip. If you want to build flvtoh264 you need to either make all or make flvtoh264.

linuxstb commented 11 years ago

Thanks for this - it's definitely something I will be happy to merge, I just need to take a closer look when I have some time.

Regarding libcec, there was a bug in the C API in 2.0.5, which you've discovered. I submitted a PR to fix it, which was accepted, but it's not in any releases yet. As the pidvbip README says, you need to built it yourself from git.

I do have a couple of initial comments though. Firstly, the configure script has quite general "rm" commands - things like "rm config.*". I think it would be nicer if they were more specific.

Secondly, I've modified my copy of vgfont (the original has a limit of 200 chars per paragraph), so it doesn't really make sense for users to build it with the external copy. I would suspect that once I start working more on the OSD code, I will make more changes to vgfont, or possibly replace it with something else. So I'm not sure it's worth doing anything in the configure script related to that.

saintdev commented 11 years ago

I was aware of libcec. It was getting late and I hadn't had a chance to test with git HEAD yet.

Thanks for pointing that out. The config.* was unintentional, however conftest* was intentional. Anything with that name is prone to getting overwritten by the configure script anyway.

I hadn't noticed about vgfont. I've removed the option for an external vgfont. With a recent version of vgfont it would have only used the internal copy anyway, the gx_graphics_init() parameters have been changed (the reason I chose to check for that function).

linuxstb commented 11 years ago

Hi,

I've just tested this, and it initially failed, due to the use of bash extensions - in Raspbian, /bin/sh is a symlink to dash.

The problem is this line:

for path in ${VC_SDK_PATH}/include{,/interface/{vcos/pthreads,vmcs_host/linux}}; do

I would suggest just changing that for loop to a simple assignment, which IMO is clearer anyway:

PI_CFLAGS="-I${VC_SDK_PATH}/include -I${VC_SDK_PATH}/include/interface/vcos/pthreads -I${VC_SDK_PATH}/include/vmcs_host/linux"

Can I also suggest you output the values of "arch" and "debug" at the end of the script?

Thanks.

saintdev commented 11 years ago

Sorry about the bashisms. All my systems are bash, so sometimes I forget ;)

'arch' isn't that useful. It's only has two values "auto" or "rpi". It's only useful internally to the script, it was used to determine if the check for an external vgfont needed to be done. I thought it might still be useful, so I left it in. Currently, if you're not on a Pi, you would get something like: arch: auto/rpi If we used config.guess to fill it out on more systems, it may be more useful to display it. I'm guessing what you're wanting is something more like: raspberrypi: yes/no