Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Jmrunge/image support #198

Closed
wants to merge 16 commits into from
Closed

Jmrunge/image support #198

wants to merge 16 commits into from

Conversation

jmrunge
Copy link
Member

@jmrunge jmrunge commented Jan 2, 2014

Added support for scrapping images.
Depends on inaes-tic/mbc-common#42
Needs inaes-tic/mbc-mosto#144 to work with mosto

@jmrunge
Added media types object
0dc27d6 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge jmrunge mentioned this pull request Jan 2, 2014
Open
josx
josx reviewed Jan 3, 2014
View reviewed changes
media_types.js
getSeconds: function(type) {
return _.findWhere(this.types, {type: type}).seconds;
}
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe in the future we can use something like node-mime
https://github.com/broofa/node-mime

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its nice, but I dont know if all the image or video types identified by node-mime are supported by ffmpeg. And as for using our custom types file, I think perhaps is better to use then our own "mime" handling. Sure it should be moved to common in the very near future so mosto and caspa use the same code.

jmrunge added 11 commits January 8, 2014 12:07
@jmrunge
Modified ffmpeg to use seconds for screenshot from options if set
9e85773 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Added support for handling image files on scrape
178e2e0 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Show type on media attributes
786a1be 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Allow pieces to be clickable (so modal window with attrs to edit can …
02bb58c 
…be shown)

Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Add type to attrs to show in row mode
649db4c 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Add modal target for showing modal dialogs
08df79b 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Added image edit template
2928804 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Export image edit template
d98ab78 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Added ImageEditPrompt
f2e1345 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Added imageEdit function
b19c3c1 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Added itemEdit target for editing Pieces
95d11c0 
Signed-off-by: Juan Martin Runge <[email protected]>
@pardo-bsso
Copy link
Member

I'm mostly okay with this.
Some minor things that I can take care of after merging:

  • The modal does not show the current assigned time.
  • In models/Media.js toMilliseconds() we expect the time to be like HH:MM:SS.ss and it breaks when otherwise. We need to sanitize it here.
  • After changing the time I do not see it reflected on the playlist duration.

@jmrunge
Copy link
Member Author

jmrunge commented Jan 10, 2014

2014/1/10 Adrián Pardini [email protected]

I'm mostly okay with this.
Some minor things that I can take care of after merging:

If you can solve them before merging it would be great! I'll be online
back in about 2 hours. Ping you then. Thanks!

  • The modal does not show the current assigned time.

Ups! My bad! Didnt thought about it.

  • In models/Media.js toMilliseconds() we expect the time to be like
    HH:MM:SS.ss and it breaks when otherwise. We need to sanitize it here.

Should be normalized when setting it to the piece in the "ok" function
callback.

  • After changing the time I do not see it reflected on the playlist
    duration.

Saw it just a few minutes ago. Couldnt find quick where to fix it...

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/198#issuecomment-32041898
.

@pardo-bsso
Copy link
Member

2014/1/10 Adrián Pardini [email protected]
I'm mostly okay with this. Some minor things that I can take care of after merging:

If you can solve them before merging it would be great! I'll be online back in about 2 hours. Ping you then. Thanks!

While I think most of them won't take longer some things I'm doing affect all the views. I'd rather merge this and then focus on that.

  • The modal does not show the current assigned time.

Ups! My bad! Didnt thought about it.

no biggie.

  • In models/Media.js toMilliseconds() we expect the time to be like HH:MM:SS.ss and it breaks when otherwise. We need to sanitize it here.

Should be normalized when setting it to the piece in the "ok" function callback.

Here whatever I put on the input ends up on the duration field. (It accepts for example aa:bb:ccdsafjsadlfkjdsalfkj)

  • After changing the time I do not see it reflected on the playlist duration.

Saw it just a few minutes ago. Couldnt find quick where to fix it...

Neither I.

@jmrunge
Make Piece to show its length on edit prompt
4cc5880 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Add alert function
81893e9 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Validate length input format
7acdcbe 
Signed-off-by: Juan Martin Runge <[email protected]>
@jmrunge
Copy link
Member Author

jmrunge commented Jan 13, 2014

2014/1/10 Adrián Pardini [email protected]

2014/1/10 Adrián Pardini [email protected]
I'm mostly okay with this. Some minor things that I can take care of after
merging:

If you can solve them before merging it would be great! I'll be online
back in about 2 hours. Ping you then. Thanks!

While I think most of them won't take longer some things I'm doing affect
all the views. I'd rather merge this and then focus on that.

  • The modal does not show the current assigned time.

Ups! My bad! Didnt thought about it.

no biggie.

Done!

  • In models/Media.js toMilliseconds() we expect the time to be like
    HH:MM:SS.ss and it breaks when otherwise. We need to sanitize it here.

    Should be normalized when setting it to the piece in the "ok" function
    callback.

Here whatever I put on the input ends up on the duration field. (It
accepts for example aa:bb:ccdsafjsadlfkjdsalfkj)

Done!

  • After changing the time I do not see it reflected on the playlist
    duration.

    Saw it just a few minutes ago. Couldnt find quick where to fix it...

Neither I.

Done!

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/198#issuecomment-32047035
.

@pardo-bsso pardo-bsso mentioned this pull request Jan 14, 2014
Open
@josx josx closed this Mar 11, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jmrunge @pardo-bsso @josx