Add basic check for libdvdrad, libdvdnav, libdvdcss #7

Open
deloptes wants to merge 1 commits from issue/1/kplayer into master
Collaborator

these libraries are required to build kplayer with DVD support in various flavours.

The libraries are available from debian multimedia or directly from the VLAN/VLC project.

Signed-off-by: Emanoil Kotsev deloptes@gmail.com

PS: I am sure it is not perfect, but it is the basic idea to check availability and stop building or perhaps issue just a warning. I am not sure what is better.

From my experience it is better to not build without the libraries as there is lack of functionality (extended DVD support)

This is why I bound this to WITH_ALL_OPTIONS.

these libraries are required to build kplayer with DVD support in various flavours. The libraries are available from debian multimedia or directly from the VLAN/VLC project. Signed-off-by: Emanoil Kotsev <deloptes@gmail.com> PS: I am sure it is not perfect, but it is the basic idea to check availability and stop building or perhaps issue just a warning. I am not sure what is better. From my experience it is better to not build without the libraries as there is lack of functionality (extended DVD support) This is why I bound this to WITH_ALL_OPTIONS.
Owner

Since the libraries are not part of a default distribution, I don't think we should stop building if those libraries are not found.

Having said that, it is good to check for their presence and eventually display a warning message that some functionality would be missing due to the lack of some libraries.

Since the libraries are not part of a default distribution, I don't think we should stop building if those libraries are not found. Having said that, it is good to check for their presence and eventually display a warning message that some functionality would be missing due to the lack of some libraries.
Poster
Collaborator

OK, thank you. I'll change the behavior.

OK, thank you. I'll change the behavior.
Owner

Here, it is perfectly fine that if WITH_xxx="ON", then failure must occur if the option cannot be met. And it is correct that the option WITH_xxx="OFF" must be used for such cases.

@deloptes now you leave it in the current state, I'll make comments there during the review.

Here, it is perfectly fine that if `WITH_xxx="ON"`, then failure must occur if the option cannot be met. And it is correct that the option `WITH_xxx="OFF"` must be used for such cases. @deloptes now you leave it in the current state, I'll make comments there during the review.
Poster
Collaborator

OK Slavek,

when looking how others do the checks I saw one configuration (pretty comprehensive) which was doing a download of the sources for those packages. I am not sure why Debian do not have them, but without them kplayer can not handle the DVD. Mplayer however can open the same dvd.

What I understood is that those are required for encrypted dvds or something like this. My understanding is that kplayer is depenent on the libraries to handle dvds.

I was also thinking to do some research if they were part of debian or how did it happend that they are no maintained by VLC.

What I am trying to say is that perhaps kplayer can handle DVDs (because mplayer does), but by historiacl precedence now the libraries are missing.

I would be glad if you share your view, what one could or should do in such a case.

thanks

OK Slavek, when looking how others do the checks I saw one configuration (pretty comprehensive) which was doing a download of the sources for those packages. I am not sure why Debian do not have them, but without them kplayer can not handle the DVD. Mplayer however can open the same dvd. What I understood is that those are required for encrypted dvds or something like this. My understanding is that kplayer is depenent on the libraries to handle dvds. I was also thinking to do some research if they were part of debian or how did it happend that they are no maintained by VLC. What I am trying to say is that perhaps kplayer can handle DVDs (because mplayer does), but by historiacl precedence now the libraries are missing. I would be glad if you share your view, what one could or should do in such a case. thanks
Owner

The main question here (and sorry if I didn't explain well in my previous comment) is that those libraries are not part of a stadard (debian) distro.

Having an option to enable/disable the use of those libraries is good, but the question is: are we going to enable the use of those libraries by default (as in WITH_ALL_OPTIONS)? Do not forget that build environment and user installation environment will be different, so if we enable those libraries by default, the user will have to add additional repositories to their apt sources. So we need at least a way to let the user know, otherwise he won't be able to install kplayer.

The main question here (and sorry if I didn't explain well in my previous comment) is that those libraries are not part of a stadard (debian) distro. Having an option to enable/disable the use of those libraries is good, but the question is: are we going to enable the use of those libraries by default (as in WITH_ALL_OPTIONS)? Do not forget that build environment and user installation environment will be different, so if we enable those libraries by default, the user will have to add additional repositories to their apt sources. So we need at least a way to let the user know, otherwise he won't be able to install kplayer.
Poster
Collaborator

Hi,
could be that libdvdcss2 is optional - and perhaps for runtime, because I can uninstall it without uninstalling kplayer.

With libdvdnav4 and libdvdread4 installed and compiled all of TDE today, I can see following when trying to remove one of them.

The following packages will be REMOVED:
  k3b-i18n-trinity* k3b-trinity* kplayer-trinity* libarts1-xine-trinity* libdvdnav4* libdvdread4* libk3b3-extracodecs-trinity* libk3b3-trinity* libxine2*
  libxine2-misc-plugins* libxine2-plugins* mplayer* tde-trinity* tdemultimedia-trinity*
0 upgraded, 0 newly installed, 14 to remove and 0 not upgraded.
After this operation, 59.4 MB disk space will be freed.

This is why I asked on the list how one can find out what exactly gets configured.

It is also the question if one should not maintain a local copy of these libraries and this is why I asked those questions here.

Michele seems to be correct here in saying that if enabled and compiled with, it would force the user to install foreign packages.

For me those are candidates for edeps. I do not know why k3b or arts or xine picks them :/.

regards

Hi, could be that libdvdcss2 is optional - and perhaps for runtime, because I can uninstall it without uninstalling kplayer. With libdvdnav4 and libdvdread4 installed and compiled all of TDE today, I can see following when trying to remove one of them. ``` The following packages will be REMOVED: k3b-i18n-trinity* k3b-trinity* kplayer-trinity* libarts1-xine-trinity* libdvdnav4* libdvdread4* libk3b3-extracodecs-trinity* libk3b3-trinity* libxine2* libxine2-misc-plugins* libxine2-plugins* mplayer* tde-trinity* tdemultimedia-trinity* 0 upgraded, 0 newly installed, 14 to remove and 0 not upgraded. After this operation, 59.4 MB disk space will be freed. ``` This is why I asked on the list how one can find out what exactly gets configured. It is also the question if one should not maintain a local copy of these libraries and this is why I asked those questions here. Michele seems to be correct here in saying that if enabled and compiled with, it would force the user to install foreign packages. For me those are candidates for edeps. I do not know why k3b or arts or xine picks them :/. regards
Owner

Rather than using additional edeps, it may be enough to add a note in debian/control file that explains where to find the additional libraries (i.e. pointing to debian multimedia for example. So if a user tries to install it without having the additional repo, it would fail and he could get info from the package description.

Anyhow it would still be an annoyance for those who do not want those libraries and are happy to have kplayer without DVD support.

Rather than using additional edeps, it may be enough to add a note in debian/control file that explains where to find the additional libraries (i.e. pointing to debian multimedia for example. So if a user tries to install it without having the additional repo, it would fail and he could get info from the package description.<br/> Anyhow it would still be an annoyance for those who do not want those libraries and are happy to have kplayer without DVD support.
Owner

The libdvdnav4 and libdvdread4 packages are part of the standard distribution:

(buster-amd64-sbuild)root@fenrir:~# apt-cache policy libdvdnav4
libdvdnav4:
  Installed: (none)
  Candidate: 6.0.0-1
  Version table:
     6.0.0-1 500
        500 http://deb.debian.org/debian buster/main amd64 Packages
(buster-amd64-sbuild)root@fenrir:~# apt-cache policy libdvdread4
libdvdread4:
  Installed: (none)
  Candidate: 6.0.1-1
  Version table:
     6.0.1-1 500
        500 http://deb.debian.org/debian buster/main amd64 Packages

And libdvdcss2 should be optional => package is not a direct dependency. So there should be no problem. I'll look into it in more detail during the review.

The libdvdnav4 and libdvdread4 packages are part of the standard distribution: ``` (buster-amd64-sbuild)root@fenrir:~# apt-cache policy libdvdnav4 libdvdnav4: Installed: (none) Candidate: 6.0.0-1 Version table: 6.0.0-1 500 500 http://deb.debian.org/debian buster/main amd64 Packages (buster-amd64-sbuild)root@fenrir:~# apt-cache policy libdvdread4 libdvdread4: Installed: (none) Candidate: 6.0.1-1 Version table: 6.0.1-1 500 500 http://deb.debian.org/debian buster/main amd64 Packages ``` And libdvdcss2 should be optional => package is not a direct dependency. So there should be no problem. I'll look into it in more detail during the review.
Poster
Collaborator

Ah, thank you Slavek.

So the problem was that as I wrote to the list kplayer can be compiled with or without libdvdread/nav which disables the DVD part.

After I installed those and recompiled the DVD feature is there. Only libdvdcss is optional hense the extra conditions in the cmake checks.

This means one will get kplayer with dvdread/nav by default and if one wants dvdcss one should just install libdvdcss, but kplayer must be build with libdvdcss for this to work. Correct?

I just tested and I can remove/install libdvdcss with no dependency on kplayer. kplayer seems to work as before. Perhaps we need to verify in the code how this is handled.

Ah, thank you Slavek. So the problem was that as I wrote to the list kplayer can be compiled with or without libdvdread/nav which disables the DVD part. After I installed those and recompiled the DVD feature is there. Only libdvdcss is optional hense the extra conditions in the cmake checks. This means one will get kplayer with dvdread/nav by default and if one wants dvdcss one should just install libdvdcss, but kplayer must be build with libdvdcss for this to work. Correct? I just tested and I can remove/install libdvdcss with no dependency on kplayer. kplayer seems to work as before. Perhaps we need to verify in the code how this is handled.
Poster
Collaborator

I just tested with one original DVD and without libdvdcss it does not play.

I installed libdvdcss and it plays, so it looks like a clever runtime dependency, that we can live with.

AFAIR libdvdcss was delivering encrypted content, but not sure ATM and can't double check.

I hope this information helps.

I just tested with one original DVD and without libdvdcss it does not play. I installed libdvdcss and it plays, so it looks like a clever runtime dependency, that we can live with. AFAIR libdvdcss was delivering encrypted content, but not sure ATM and can't double check. I hope this information helps.
Owner

We can handle dependency in debian/control file, that is easy to do. The main point will be whether we want to build with or withour libdvdcss support.

Alternatively we would have to think of something like a plugin style support for libdvdcss, so that we could have a main kplayer that does not require libdvdcss and an optional plugin with depends on libdvdcss and if installable but not mandatory

We can handle dependency in debian/control file, that is easy to do. The main point will be whether we want to build with or withour libdvdcss support. Alternatively we would have to think of something like a plugin style support for libdvdcss, so that we could have a main kplayer that does not require libdvdcss and an optional plugin with depends on libdvdcss and if installable but not mandatory
Owner

I suppose libdvdcss2 is not needed in building. It works similarly with Kaffeine – libdvdcss2 library is not needed during building, it is not used directly from kaffeine, but probably from libdvdnav4 or libdvdread4.

I suppose libdvdcss2 is not needed in building. It works similarly with Kaffeine – libdvdcss2 library is not needed during building, it is not used directly from kaffeine, but probably from libdvdnav4 or libdvdread4.
Poster
Collaborator

Most probably true. I had the problem that in general it was not taking care of dvdread and dvdnav, so originally it looks like after moved to smake those were ignored.

May be I can double check this later and report back.

I will remove libdvdcss and build with libdvdcss enabled. It should work when libdvdcss is installed.

Most probably true. I had the problem that in general it was not taking care of dvdread and dvdnav, so originally it looks like after moved to smake those were ignored. May be I can double check this later and report back. I will remove libdvdcss and build with libdvdcss enabled. It should work when libdvdcss is installed.
Owner

Hi Emanoil, what is the status of this PR? Do you still need to do more work on this or should we look at it for merging?

Hi Emanoil, what is the status of this PR? Do you still need to do more work on this or should we look at it for merging?
Poster
Collaborator

Hi,
my impression was sthat Slavek wanted to have a look there regarding options.

From my perspective it is complete for now, but I am not a master in cmake. I am also not sure - the main question was what to do with libdvd(read|nav|css).

Hi, my impression was sthat Slavek wanted to have a look there regarding options. From my perspective it is complete for now, but I am not a master in cmake. I am also not sure - the main question was what to do with libdvd(read|nav|css).
Owner

oh ok, I wasn't sure of the status given the discussion above, which is why I asked 😄

oh ok, I wasn't sure of the status given the discussion above, which is why I asked :smile:
Poster
Collaborator

I don't recall the details. I think we discussed in the dev UG.

The questionfrom my side was what needs to be done to build with libdvdcss. I recall testing without libdvdcss2-dev installed, but after build it was not working. So it is definitely needed for some reason. Looking at the linked libraries, I do not see neither libdvd, nor libdvdcss, but could be that code is enabled internally with dlopen.

I've never looked closer.

I don't recall the details. I think we discussed in the dev UG. The questionfrom my side was what needs to be done to build with libdvdcss. I recall testing without libdvdcss2-dev installed, but after build it was not working. So it is definitely needed for some reason. Looking at the linked libraries, I do not see neither libdvd, nor libdvdcss, but could be that code is enabled internally with dlopen. I've never looked closer.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TDE/kplayer#7
Loading…
There is no content yet.