Thumbnails for files with non 3-letter extensions

rpabel

Joined: 2006-12-05
Posts: 6
Posted: Mon, 2012-04-30 22:15

Hi,
I submitted this topic to the "Troubleshooting and Problems" forum 5 days ago, but I guess it was misplaced, it's more like a feature request/bug report. Since nobody replied to that post, I submit it again here. I apologize if this is considered "wrong" behavior.
I've been adding my gallery (Version 3.0.3) with many pictures and videos lately.
I added the file extension "m2ts" to the accepted file extension list of the video module and they got imported
properly, just the preview thumbnail was not displayed at all. But when the video thumbnail was used as the album cover,
it was displayed fine (for the album, not the video file).
I noticed the preview file was given a funny file extension on disk: "mjpg".
Even direct access to this file failed, I guess because it is not in the "allowed_filetypes" format list.
I tracked the problem to these pieces of code:

return preg_replace("/...$/", "jpg", $base);

which can be found in modules/gallery/helpers/graphics.php, line 159, and modules/gallery/models/item.php, lines 191 and 216 .
Since this clearly this assumes a 3-letter file extension, it will produce 'funny' results for any other type.
I changed these occurances to:

$replace = '/' . pathinfo($base, PATHINFO_EXTENSION) . "$/";
return preg_replace( $replace, "jpg", $base);

and now thumbnails with m2ts videos works fine. (Better solutions - substring replacements - are probably possible).
The problem is: The above 3-letter substitution is also found in modules, most notably the video module (videos/models/item.php: lines 191 and 217), probably others too.
Maybe this should therefore be implemented as an independent function, but I'd at least hope this could get changed
for the gallery core.
Thanks,
Roland

 
floridave
floridave's picture

Joined: 2003-12-22
Posts: 25953
Posted: Tue, 2012-05-01 03:02

feature request/bug report should be created:
https://sourceforge.net/apps/trac/gallery/newticket
so it does not get lost in the forums.

I understand your enthusiasm but we aim for making it work for most 80/20 of the user base and to be honest in the 8+ years of using gallery I have never even heard of m2ts. It is a very small edge case... did it even work in G2 where we went overboard supporting different file types?

Dave

_____________________________________________
Blog & G2 || floridave - Gallery Team

 
rpabel

Joined: 2006-12-05
Posts: 6
Posted: Tue, 2012-05-01 10:52

Hi,
I don't know about G2, I only had my new camera for a year now.
According to Wikipedia, m2ts is a valid "Blu-ray Disc Audio-Video (BDAV) MPEG-2 Transport Stream (M2TS)" container file format.
I am a little mystified that people uploading "*.mpeg" files never experienced this problem. The "allowed_extensions" string by default contains "mpeg". Maybe "mpeg" is already so outdated that no device uses it any more.
Three letter file extension are of course the common case, but there are other types out there.
Although it probably is really just a problem for a few people, I don't see the code change hurting anyone (but I'm not a big PHP developer).
Will create the ticket.
Roland

 
floridave
floridave's picture

Joined: 2003-12-22
Posts: 25953
Posted: Tue, 2012-05-01 15:00

Don't know how 'Blu-ray Disc Audio-Video (BDAV) MPEG-2 Transport Stream (M2TS)' is in relation to "Your Photos on Your Website"
At least one of the devs will see it and make a call. It is a slippery slope when we start adding things that are not in the scope of the product. We end up with bloated and hard to maintain software. I know you don't see the code change hurting anyone but then we open the door for ME TO and then.....

Dave
_____________________________________________
Blog & G2 || floridave - Gallery Team

 
jnash
jnash's picture

Joined: 2004-08-02
Posts: 605
Posted: Tue, 2012-05-01 15:24

Floridave: I agree somewhat, but it seems that the fix for this would be an improvement to the core, as it would 'fit' much better for the intended operation.

rpabel, I'd say file a 'feature/bug' request and see where it goes.

Unfortunately, as you stated rpabel, there are a lot of modules that use this function and they are coded in to the module, that will be the tricky part. However, I'm guessing that if your hit by the 'bug' then you can always contact the module author for a fix.

Just my 1.3 cents (economy still not good enough to give 2cents)

 
rpabel

Joined: 2006-12-05
Posts: 6
Posted: Tue, 2012-05-01 16:04

Yes, it's a video file. Why support videos at all if all you want for G3 to do is display "Your Photos on Your Website"?
I really like the Gallery, have been using it for years. If videos are not supported, that would be a K.O. criterion for me.
I would not set up another web service so that I can also present this one video along the hundreds of photos I made at one evening.
I have about 13K photos, less than 50 videos. But a photo just can't cut it when my little brother sings drunk...
And to the opening of the door: The intent of the code seemed to me to replace the file extension. The original author assumed 3-letter extensions, which is ok in 99.9% of all cases. But if one of my students had turned this in as an exercise, I'd deduct a point. If you ask me, a separate method (which can check & sanitize the input, since it is user generated data) to change the file extension would be far more appropriate.
Roland

 
jnash
jnash's picture

Joined: 2004-08-02
Posts: 605
Posted: Tue, 2012-05-01 16:48

rpable, my suggestion is to file a bug report, and include your fixes/suggestions. After all, this is an open source project, and thus, survives on community input. I don't think anyone is arguing your suggestions now.

Post here:
https://sourceforge.net/apps/trac/gallery/newticket

and see what happens.

 
floridave
floridave's picture

Joined: 2003-12-22
Posts: 25953
Posted: Sun, 2012-05-06 02:43

Fixed in this commit:
https://github.com/gallery/gallery3/commit/ef4dbd18af218a3c68a776122108af4b0d0191a4

Dave
_____________________________________________
Blog & G2 || floridave - Gallery Team