google / sagetv

SageTV is a cross-platform networked DVR and media management system
http://forums.sagetv.com/
Apache License 2.0
267 stars 174 forks source link

Fixed 32-bit Windows USB-UIRT update install issue. #451

Closed wnjj closed 3 years ago

wnjj commented 3 years ago

Fixed bug where the Windows installer would remove the existing uu_irsage.dll and not replace it. Was caused by original V7 dll having version info and V9 not having it. Modified .gitignore to reduce the large amount of untracked clutter. Intermediate file path cleanup in launcher VS project.

wnjj commented 3 years ago

It looks like the Gradle wrapper version needs to be newer to support Java 9. https://stackoverflow.com/questions/48851145/could-not-determine-java-version-9-0-4

This is set in gradle-wrapper.properties.

Narflex commented 3 years ago

It looks like the Gradle wrapper version needs to be newer to support Java 9. https://stackoverflow.com/questions/48851145/could-not-determine-java-version-9-0-4

This is set in gradle-wrapper.properties.

Fixed that, then fixed the compatibility of the build file...then ran into an mplayer compile issue..fixed that...maybe it'll all build now in travis. :)

wnjj commented 3 years ago

It looks like the Gradle wrapper version needs to be newer to support Java 9. https://stackoverflow.com/questions/48851145/could-not-determine-java-version-9-0-4 This is set in gradle-wrapper.properties.

Fixed that, then fixed the compatibility of the build file...then ran into an mplayer compile issue..fixed that...maybe it'll all build now in travis. :)

Looks like there are still some linker dependency issues:

libavcodec/libavcodec.a(snow.o): In function encode_q_branch': snow.c:(.text+0x5eae): undefined reference toff_epzs_motion_search' snow.c:(.text+0x5f3b): undefined reference to ff_get_mb_score' liba52/liba52.a(imdct.o): In functionimdct_do_512_sse': imdct.c:(.text+0xc09): undefined reference to sseSinCos1c' imdct.c:(.text+0xc17): undefined reference tosseSinCos1d' imdct.c:(.text+0xdf1): undefined reference to sseSinCos1c' imdct.c:(.text+0xdf8): undefined reference tosseSinCos1d' imdct.c:(.text+0xe3a): undefined reference to sseWindow' imdct.c:(.text+0xe9e): undefined reference tosseWindow' imdct.c:(.text+0xeea): undefined reference to sseWindow' imdct.c:(.text+0xf2e): undefined reference tosseWindow' collect2: error: ld returned 1 exit status

Narflex commented 3 years ago

It looks like the Gradle wrapper version needs to be newer to support Java 9. https://stackoverflow.com/questions/48851145/could-not-determine-java-version-9-0-4 This is set in gradle-wrapper.properties.

Fixed that, then fixed the compatibility of the build file...then ran into an mplayer compile issue..fixed that...maybe it'll all build now in travis. :)

Looks like there are still some linker dependency issues:

libavcodec/libavcodec.a(snow.o): In function encode_q_branch': snow.c:(.text+0x5eae): undefined reference toff_epzs_motion_search' snow.c:(.text+0x5f3b): undefined reference to ff_get_mb_score' liba52/liba52.a(imdct.o): In functionimdct_do_512_sse': imdct.c:(.text+0xc09): undefined reference to sseSinCos1c' imdct.c:(.text+0xc17): undefined reference tosseSinCos1d' imdct.c:(.text+0xdf1): undefined reference to sseSinCos1c' imdct.c:(.text+0xdf8): undefined reference tosseSinCos1d' imdct.c:(.text+0xe3a): undefined reference to sseWindow' imdct.c:(.text+0xe9e): undefined reference tosseWindow' imdct.c:(.text+0xeea): undefined reference to sseWindow' imdct.c:(.text+0xf2e): undefined reference tosseWindow' collect2: error: ld returned 1 exit status

Yeah, I saw those...the ones in snow.c are easy to fix I think (and disabling snow would be fine too). But the ones in imdct.c I'm not sure about...I'm not very familiar with ASM code...do you know what's wrong there? I guess we could also just disable the optimized ASM version of that function and then it should build fine (and I'm not really worried about performance for something like AC3 decoding).

Narflex commented 3 years ago

Thanks for the installer fixes! Hopefully we can get Travis working again too. :)

wnjj commented 3 years ago

Yeah, I saw those...the ones in snow.c are easy to fix I think (and disabling snow would be fine too). But the ones in imdct.c I'm not sure about...I'm not very familiar with ASM code...do you know what's wrong there? I guess we could also just disable the optimized ASM version of that function and then it should build fine (and I'm not really worried about performance for something like AC3 decoding).

Looking in the source file, it would mean ARCH_X86 or ARCH_X86_64 are not defined. Those are supposed to be setup by mplayer's 'configure' but maybe a change in compiler versions broke that? I'll look a little more into it.

wnjj commented 3 years ago

Yeah, I saw those...the ones in snow.c are easy to fix I think (and disabling snow would be fine too). But the ones in imdct.c I'm not sure about...I'm not very familiar with ASM code...do you know what's wrong there? I guess we could also just disable the optimized ASM version of that function and then it should build fine (and I'm not really worried about performance for something like AC3 decoding).

Looking in the source file, it would mean ARCH_X86 or ARCH_X86_64 are not defined. Those are supposed to be setup by mplayer's 'configure' but maybe a change in compiler versions broke that? I'll look a little more into it.

configure creates config.h which contains the defines needed by the code later. The log says that arch 'x86_64' was detected which in turn should write "#define ARCH_X86_64 1" into config.h, which is included by mplayer source. Is there any way to see the config.h file the Travis compile created?

Narflex commented 3 years ago

I don't think config.h matters. If you look at the usage of the undefined symbols in imdtc.c you'll see that they are defined and also only used within blocks surrounded by the ARCH_X86 or ARCH_X86_64 guards. So I think those defines are correct. My suspicion is there's something different related to that symbol usage within ASM code.

wnjj commented 3 years ago

You're correct and I was looking in the wrong place. What all of the undefined use cases have in common is that they are in the form var(%reg) which is a 'var' offset from the 'reg' value. All other uses of static variables are as direct values. Is the compiler newer than it used to be or why did mplayer compile break recently?

Narflex commented 3 years ago

The compiler got changed when I updated the Ubuntu version to deal with the distro being too old...maybe I should just go back and fix that a different way (I think I just needed to update the apt sources list). That will likely be easier to do and not require me to test any changes in the code. 🙂

wnjj commented 3 years ago

I think I found something to try. Consider adding “attribute_used” in the declaration of sseSinCos1c, sseSinCos1d and sseWindow like the variable ps111_1 has. I’ve read that forces the compiler to keep symbols when they are used inside asm. It’s likely the newer compiler is more aggressive about cleanup of what it thinks are unused variables. Those variables are only set in the c code and not used on the right hand side so it thinks they are unused.

It’s looks like that attribute is needed for gcc > ver 3.0. https://github.com/google/sagetv/blob/aff35f9cba3dd6a9ed8f8a7a5ca2b93e6dc56fa2/third_party/mplayer/libavutil/internal.h#L30

Here’s where I found the info. https://stackoverflow.com/questions/8810390/how-to-use-a-global-variable-in-gcc-inline-assembly#8811828

wnjj commented 3 years ago

For the 2 undefined references in snow.c, remove the “inline” modifier in the mpegvideo.h header file. Inline is already used when the function is defined. I noticed that ffmpeg’s version of this same file does not have inline in the header. I suspect that causes the compiler to omit the symbol since it knows the function will created inline. https://github.com/google/sagetv/blob/aff35f9cba3dd6a9ed8f8a7a5ca2b93e6dc56fa2/third_party/mplayer/libavcodec/mpegvideo.h#L770

Narflex commented 3 years ago

I saw somewhere else where they added extern for the mpegvideo.h files to fix that problem upstream. I'm trying that now along with your other suggestion about attribute_used. Actually made a pull request (first time I've done that on GitHub, usually I just commit to master directly) so I can be sure this works before I commit it.

Narflex commented 3 years ago

Hey, it worked! Thanks for the help! Now I need to move this to travis-ci.com so that it keeps working past the end of May. :)

wnjj commented 3 years ago

I see that, excellent! It took some searching but that attribute thing makes sense. It probably was always needed but optimization settings on the compiler may have been lower before.

I have some initial success scripting GitHub release builds in a test repo. I hope to have that all going in the near future so the Linux releases can be pushed out automatically like they were for Bintray.

Narflex commented 3 years ago

Sounds great! That'd be awesome to have automated Linux releases. Thanks again so much for your contributions. :)