No problems reported, but output file is not generated.

Comments

arthurf’s picture

Is this from media mover or just testing with ffmepg_wrapper? Are other conversions working with ffmpeg_wrapper?

iva2k’s picture

Status: Active » Needs review
StatusFileSize
new7.77 KB

Here's a patch that fixes ffmpeg thumbnails and adds some error checking and more detailed reporting.

iva2k’s picture

Project: FFmpeg Wrapper » Media Mover
iva2k’s picture

Video conversion (kinda) works. There are few glitches with ffmpeg library. If I find something material I will post a patch to either ffmpeg wrapper or mm_ffmpeg.

arthurf’s picture

Video conversion should be working with media mover and ffmpeg- provided the version you're using matches up with where ffmpeg_wrapper is- the module needs to be updated to deal with changes to the arguments. Unfortunately ffmpeg_wrapper is poorly architected- I think it needs a serious look to restructure it so it can handle differing versions of ffmpeg more graceully.

iva2k’s picture

I did not touch video part of mm_ffmpeg module in the above patch. I made video work by providing custom args to ffmpeg to cover the bugs in the ffmpeg. In my sandbox I added more detailed error handling which helped me debug the conversion part, but it did not change the actual functionality. If you want, I can post the video patch here (or in a new issue).

Version handling of ffmpeg would be nice, but I want to get an end-to-end solution first, then I can help with ffmpeg_wrapper. Sounds like using class factory to wrap ffmpeg API can work transparently. I like what was done as an API in phpvideotoolkit (used in video.module). All settings have class methods and command is built from the object parameters. ffmpeg_wrapper already has a class wrapper, so a lot of code can be copy-pasted (if the license allows it).

arthurf’s picture

Mind posting your patch? I tested with mov and 3gp files and didn't have any issues.

iva2k’s picture

StatusFileSize
new17.46 KB

I posted ffmpeg patch on [#1926486#]. It fixes few bugs that may be affecting mm_ffmpeg conversions.

Here's a patch for mm_ffmpeg with the following:

  • incorporates thumbnails patch from above
  • adds more detailed error handling for video
  • sets new 'overwrite_output' param (it may be not necessary as it probably checks the output filename is unique)
  • adds few helper methods to class_media_mover_file: load_info(), get_step_file(), get_uri(), get_url(), get_link(), get_entity_link() (mm_ffmpeg changes use file->get_entity_link() method)
  • couple of code fixes I noted in the vicinity
iva2k’s picture

StatusFileSize
new17.91 KB

Patch in #8 has one typo, so don't use it. Here's a fixed one, get_entity_link() is improved. It will be required for the entity support patch I'm preparing for the core in #1926464: mm_fields for Drupal 7.

BTW, why the test bot is stuck in all issues in review?

iva2k’s picture

Title: FFMpeg thumbnail step silently fails to generate thumbnails » FFMpeg - better error handling, thumbnail operation fixes
arthurf’s picture

Ok this looks pretty good. Can you break out the work you did on class_media_mover_files separately? A few random thoughts: seems like what you're doing in load_info() should be done in load_info(). Can you explain what you need get_step_file() for? It seems reasonable but I'm not sure what it would get used for.

iva2k’s picture

> Can you break out the work you did on class_media_mover_files separately?
I can, but 1). I'm really behind my set schedule and the time window when I can work on drupal will close very soon. I want to finish mm_fields.module. 2) Changes in class_media_mover_files are needed for mm_ffmpeg patch. That's why I combined them into one patch. mm_fmpeg depends on additional error handling in media_mover_api_create_local_path().

>Can you explain what you need get_step_file() for?
It is a part of clean encapsulation of the logic that saves step progress in the files table. get_step_file() is already used in get_uri() and get_link() which provide help for displaying steps information in media_mover_ui patch I posted separately and is used in upcoming mm_fields module (there is a lot of scanning of step files and this cleans up a lot of mess that would be otherwise).

arthurf’s picture

Status: Needs review » Closed (fixed)

Committed this with some changes to the new functions in media_mover_files